Remove nsButtonBoxFrame (create XULButtonElement or so)
Categories
(Core :: XUL, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox107 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
|
Assignee | |
Comment 1•2 years ago
|
||
Create XULButtonElement instead to do the event handling. Pretty much a
straight port, this allows these elements to respect CSS display
properly (and use modern flexbox rather than old XUL layout).
|
|
Assignee | |
Comment 2•2 years ago
|
||
(To be landed right after the soft freeze is over)
One interesting consequence of my previous patch in this bug, is that
the time at which the XUL element processes the click event is different
(earlier, at bubbling phase rather than after everything).
This caused some test failures caused by the panel autohide, since now
it goes after than the native handling and the native handling
default-prevents to dispatch the XUL command.
Given we don't check key events for defaultPrevented either, it makes
sense to be consistent.
This uncovered an interesting failure, where the "show more tabs" button
in the sync menu starts closing the panel unexpectedly on click. The
right fix is to use closemenu="none" in that button, since otherwise
that button is not keyboard accessible (this is an issue on current
nightly that this patch fixes).
Depends on D157509
|
|
Assignee | |
Comment 3•2 years ago
|
||
This is needed to fix browser_ext_browserAction_click_types.js.
Before the first patch in this bug, the order of the built-in command
event and the synthetic event here was slightly different, so it was
papered over.
|
|
Assignee | |
Comment 4•2 years ago
|
||
This is the last fix needed for this.
The issue is that command events now are triggered by synthetic click
events (which was not the case before pretty much by chance, a side
effect of how these events were implemented).
If we open a panel by a command event triggered by the synthetic click
event, rather than the real keypress event, we end up not detecting that
it is really a keyboard activation, and not focusing the first navigable
element in the panel for example, which is unfortunate.
This was caught by browser_toolbarButtonKeyPress.js.
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/581bd16576a0 Remove nsButtonBoxFrame. r=smaug https://hg.mozilla.org/integration/autoland/rev/c152a51ae80c Don't check defaultPrevented for click events closing panels. r=Gijs https://hg.mozilla.org/integration/autoland/rev/1bb0cca45dad Propagate modifiers properly in PanelMultiView's key event handling. r=Gijs https://hg.mozilla.org/integration/autoland/rev/d43a9bd44619 Minor fixes to toolbar key navigation. r=Gijs,extension-reviewers,willdurand
|
||
Updated•2 years ago
|
|
||
Comment 7•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/581bd16576a0
https://hg.mozilla.org/mozilla-central/rev/c152a51ae80c
https://hg.mozilla.org/mozilla-central/rev/1bb0cca45dad
https://hg.mozilla.org/mozilla-central/rev/d43a9bd44619
|
||
Comment 8•2 years ago
|
||
Hey,
from mozregression, I believe this broke the (i) button within the profiler popup.
STR:
- If that's not done yet, add the profiler button by going to profiler.firefox.com and click the big blue button in the middle of the page.
- Open the profiler popup by clicking the arrow next to the profiler button.
- Click to the top right (i) button. It should trigger the display of the header of this popup.
But this doesn't seem to work anymore since this patch. Instead the popup is closed.
Could you please have a look?
|
|
||
Comment 9•2 years ago
|
||
|
|
Assignee | |
Comment 10•2 years ago
|
||
That was a pre-existing issue when activating that button with the keyboard. Filed bug 1791691 to track it.
Description
•