Closed Bug 1429929 Opened 7 years ago Closed 7 years ago

Remove "tabbrowser-close-tab-button" binding

Categories

(Firefox :: Tabbed Browser, task, P3)

task

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
Whiteboard: [xbl-available]
Priority: -- → P3
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
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+
(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...
(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
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
Pushed by dgottwald@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/11fd0d47649f Remove tabbrowser-close-tab-button binding. r=Gijs
Flags: needinfo?(dao+bmo)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: