Closed
Bug 1429929
Opened 7 years ago
Closed 7 years ago
Remove "tabbrowser-close-tab-button" binding
Categories
(Firefox :: Tabbed Browser, task, P3)
Firefox
Tabbed Browser
Tracking
()
RESOLVED
FIXED
Firefox 59
Tracking | Status | |
---|---|---|
firefox59 | --- | fixed |
People
(Reporter: Kwan, Assigned: dao)
References
Details
(Whiteboard: [xbl-available])
Attachments
(1 file)
https://mozilla.logbot.info/developers/20171220#c14038395-c14038406
><Gijs> Kwan|away: maybe file a followup bug to just move the handler stuff into JS and/or tabbrowser as a whole?
><Gijs> pretty sure that basically the only "point" of the handlers right now is to catch stuff before the enclosing tab does
><Gijs> and we could just update the enclosing tab's handlers to check for the close button
><Gijs> and then remove the binding
><Gijs> wheee
Updated•7 years ago
|
Whiteboard: [xbl-available]
Assignee | ||
Updated•7 years ago
|
Priority: -- → P3
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8942209 [details]
Bug 1429929 - Remove tabbrowser-close-tab-button binding.
https://reviewboard.mozilla.org/r/212474/#review218204
::: browser/base/content/tabbrowser.xml:8269
(Diff revision 1)
> + <handler event="dblclick" button="0" phase="capturing">
> + // for the one-close-button case
I'm assuming you've tested the edgecases here (ie I haven't tested this patch myself).
Was there a particular reason not to use a CDATA block here? I understand that it's perfectly valid as-is, but it'll break as soon as someone uses one of the XML chars, so I guess I normally would have added one. Up to you.
Attachment #8942209 -
Flags: review?(gijskruitbosch+bugs) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to :Gijs from comment #2)
> I'm assuming you've tested the edgecases here (ie I haven't tested this
> patch myself).
Tested except for the tab opening behavior -- we don't exercise that in most situations nowadays.
> Was there a particular reason not to use a CDATA block here?
The original handler that I moved didn't have one...
Comment hidden (mozreview-request) |
Comment 5•7 years ago
|
||
(In reply to Dão Gottwald [::dao] from comment #3)
> (In reply to :Gijs from comment #2)
> > Was there a particular reason not to use a CDATA block here?
>
> The original handler that I moved didn't have one...
Oh, d'oh. The diff just highlights the changes to the existing one for the previous method which is why I noticed. Carry on, though feel free to put a block in.
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8c2097e14828
Remove tabbrowser-close-tab-button binding. r=Gijs
Comment 7•7 years ago
|
||
Backed out for failing browser-chrome's browser_audioTabIcon.js and browser_usercontextid_tabdrop.js:
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8c2097e148288b26439cf0aa000b10a6fd82ca11&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Failure log browser_audioTabIcon.js: https://treeherder.mozilla.org/logviewer.html#?job_id=155963161&repo=autoland
[task 2018-01-12T17:08:52.248Z] 17:08:52 INFO - TEST-PASS | browser/base/content/test/general/browser_audioTabIcon.js | No muted attribute should be persisted -
[task 2018-01-12T17:08:52.249Z] 17:08:52 INFO - TEST-PASS | browser/base/content/test/general/browser_audioTabIcon.js | No muteReason property should be persisted -
[task 2018-01-12T17:08:52.250Z] 17:08:52 INFO - TEST-PASS | browser/base/content/test/general/browser_audioTabIcon.js | Clicking on mute should not change the currently selected tab -
[task 2018-01-12T17:08:52.251Z] 17:08:52 INFO - Buffered messages logged at 17:07:21
[task 2018-01-12T17:08:52.252Z] 17:08:52 INFO - Longer timeout required, waiting longer... Remaining timeouts: 1
[task 2018-01-12T17:08:52.253Z] 17:08:52 INFO - Buffered messages finished
[task 2018-01-12T17:08:52.254Z] 17:08:52 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_audioTabIcon.js | Test timed out -
[task 2018-01-12T17:08:52.255Z] 17:08:52 INFO - GECKO(1067) | MEMORY STAT | vsize 20974278MB | residentFast 1212MB
[task 2018-01-12T17:08:52.256Z] 17:08:52 INFO - TEST-OK | browser/base/content/test/general/browser_audioTabIcon.js | took 180047ms
[task 2018-01-12T17:08:52.257Z] 17:08:52 INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-01-12T17:08:52.258Z] 17:08:52 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_audioTabIcon.js | Found a tab after previous test timed out: about:blank -
Failure log browser_usercontextid_tabdrop.js: https://treeherder.mozilla.org/logviewer.html#?job_id=155963775&repo=autoland
[task 2018-01-12T17:04:21.070Z] 17:04:21 INFO - TEST-START | browser/components/contextualidentity/test/browser/browser_usercontextid_tabdrop.js
[task 2018-01-12T17:04:21.336Z] 17:04:21 INFO - TEST-INFO | started process screentopng
[task 2018-01-12T17:04:21.876Z] 17:04:21 INFO - TEST-INFO | screentopng: exit 0
[task 2018-01-12T17:04:21.876Z] 17:04:21 INFO - Buffered messages logged at 17:04:21
[task 2018-01-12T17:04:21.877Z] 17:04:21 INFO - Entering test bound
[task 2018-01-12T17:04:21.878Z] 17:04:21 INFO - Buffered messages finished
[task 2018-01-12T17:04:21.879Z] 17:04:21 INFO - TEST-UNEXPECTED-FAIL | browser/components/contextualidentity/test/browser/browser_usercontextid_tabdrop.js | uncaught exception - TypeError: event.originalTarget.getAttribute is not a function at onxbldragstart@chrome://browser/content/tabbrowser.xml:8230:13
Flags: needinfo?(dao+bmo)
Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/a3d15c0fe9a8
Backed out changeset 8c2097e14828 for failing browser-chrome's browser_audioTabIcon.js and browser_usercontextid_tabdrop.js
Comment 9•7 years ago
|
||
Another failure similar to the tabdrop one: https://treeherder.mozilla.org/logviewer.html#?job_id=155972410&repo=autoland
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 12•7 years ago
|
||
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/11fd0d47649f
Remove tabbrowser-close-tab-button binding. r=Gijs
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(dao+bmo)
Comment 13•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Updated•5 years ago
|
Type: enhancement → task
You need to log in
before you can comment on or make changes to this bug.
Description
•