Closed Bug 1790920 Opened 3 months ago Closed 2 months ago

Remove nsButtonBoxFrame (create XULButtonElement or so)

Categories

(Core :: XUL, task)

task

Tracking

()

RESOLVED FIXED
107 Branch
Tracking Status
firefox107 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

Attachments

(5 files)

No description provided.

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).

(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

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.

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
Blocks: 1791527

Hey,
from mozregression, I believe this broke the (i) button within the profiler popup.

STR:

  1. 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.
  2. Open the profiler popup by clicking the arrow next to the profiler button.
  3. 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?

Flags: needinfo?(emilio)
Regressions: 1791691

That was a pre-existing issue when activating that button with the keyboard. Filed bug 1791691 to track it.

Flags: needinfo?(emilio)
Regressions: 1792802
Regressions: 1794817
You need to log in before you can comment on or make changes to this bug.