Closed Bug 1868552 Opened 5 months ago Closed 5 months ago

-moz-user-focus handling is a mess.

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

defect

Tracking

()

RESOLVED FIXED
123 Branch
Tracking Status
firefox122 --- wontfix
firefox123 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

(Keywords: dev-doc-complete)

Attachments

(3 files)

-moz-user-focus: none should force the element not to be focusable. The current behavior is hilarious:

  • -moz-user-focus: none is the default.
  • -moz-user-focus: ignore does something weird with events but otherwise behaves as none.
  • -moz-user-focus: normal forces the XUL elements to be tabbable, but gets ignored on HTML otherwise.
  • There are a bunch of other values that do nothing.

Proposal:

  • -moz-user-focus: normal is the default.
  • -moz-user-focus: none prevents focusability.
  • -moz-user-focus: ignore should ideally be removed or aliased to none, but XUL uses it so might be tricky, we'll see.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #0)

  • -moz-user-focus: ignore should ideally be removed or aliased to none, but XUL uses it so might be tricky, we'll see.

In my understanding, none element is not focusable, but clicking it makes no element has focus. However, clicking ignore element does not make the focused element not blurred.
https://searchfox.org/mozilla-central/rev/648a427a0ffc4c62118dbb24bcd88a6b52f54d78/dom/events/EventStateManager.cpp#3522-3523

So, I think that it needs to be as-is.

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #1)

(Oh, only overrides the tab index??)

Right, and that's super-confusing.

In my understanding, none element is not focusable, but clicking it makes no element has focus. However, clicking ignore element does not make the focused element not blurred.
https://searchfox.org/mozilla-central/rev/648a427a0ffc4c62118dbb24bcd88a6b52f54d78/dom/events/EventStateManager.cpp#3522-3523

So, I think that it needs to be as-is.

Yeah I guess that the idea is that if you click on, e.g., a toolbar spacer, the urlbar isn't blurred? that's a bit weird compared to HTML, but sure, seems we need a hook for that, so fine to keep ignore.

If removing these were to become a compat issue in the wild, we could
alias them effortlessly. But honestly they're not even documented in MDN
so I'm pretty sure it should be safe to remove.

Make it be output-only, not having that confusing in-out tab-index
parameter that is special for XUL to become focusable with
-moz-user-focus: normal. Instead, do that explicitly in
nsIFrame::IsFocusable().

Also, call it IsFocusableWithoutStyle(), since that's what it is.

Depends on D195643

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7182353bf85c
Remove unused -moz-user-focus values. r=jwatt

This is tested via the inert tests, effectively, but I can add more
explicit tests.

Remove rules that would otherwise change behavior (the other rules in
the tree apply to XUL elements and serve a purpose).

Keywords: leave-open
See Also: → 1868579
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/efcb1145c2cf
Refactor nsIContent::IsFocusable for clarity. r=masayuki
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/63ab69dfad15
More precisely maintain XUL behavior on macOS.
Keywords: leave-open
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d576a857df6f
Make the initial value of -moz-user-focus normal, and make none actually prevent focusability. r=smaug,jwatt
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch
Duplicate of this bug: 1675416
Regressions: 1871745

Sheriffs, can we back out https://hg.mozilla.org/mozilla-central/rev/d576a857df6f from beta for now while we decide on a way to address bug 1871745?

Flags: needinfo?(sheriffs)
Flags: needinfo?(sheriffs)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 122 Branch → ---

Actually, it's still fixed in 123, backout was only for beta 122.

Status: REOPENED → RESOLVED
Closed: 5 months ago5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 123 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: