Closed Bug 1541088 Opened 6 years ago Closed 6 years ago

Add-Ons in More Tools menu not receiving keyboard input despite being focused

Categories

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

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 + verified
firefox68 --- verified

People

(Reporter: spamcop, Assigned: hsivonen)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached video fx-focus-fail.mp4 (deleted) —

I'm unsure for which product/category to file this bug.

I'm currently using 67.0b6 and every time I open the More Tools menu, select an add-on that displays an input text field, focus that text field (I click into the text field and see my blinking cursor there) and then start to type something, the first keystroke ends up at the last focused element of the webpage displayed, only the second one and on end up in the actually focused input field.

To make this more obvious, let me attach a screen recording.

In the screen recording you can see that the textfield on this page used to have focus, then I click on the input field of an add-on and you can see how it obtains focus (notice the blinking cursor), then I type the letter T twice but as you may notice, the first "t" appears in the textfield of the site, only the second "t" appears in the actual input field of the add-on. I can reproduce that on every side and with every add-on offering input fields.

I'm using Fx 67.0b6 on macOS 10.14.4 (18E226).

Not sure if this belongs in Toolbars and Customization but it seems like the best fit right now. To clarify, this doesn't occur if the add-on is in the main toolbar instead of in the "More tools..." dropdown?

Component: General → Toolbars and Customization
Flags: needinfo?(spamcop)

(In reply to Doug Thayer [:dthayer] from comment #1)

To clarify, this doesn't occur if the add-on is in the main toolbar instead of in the "More tools..." dropdown?

Just checked and indeed, it won't happen if the item is in the main toolbar.

Flags: needinfo?(spamcop)

I can't reproduce this with the Search bar when placed in the More Tools overflow menu, so it seems like it's related to the iframe used by add-ons.

TGOS, can you run mozregression to find when this behavior started happening? I'm curious if it ever worked. You can find out more about mozregression at https://mozilla.github.io/mozregression/

Flags: needinfo?(spamcop)
Priority: -- → P3
Priority: P3 → --

(In reply to (behind on needinfos) Jared Wein [:jaws] (please needinfo? me) from comment #3)

TGOS, can you run mozregression to find when this behavior started happening?

I cannot install mozregression, when attempting, I get:

Cannot uninstall 'six'. It is a distutils installed project and thus we cannot accurately determine which files belong to it which would lead to only a partial uninstall.

I'm curious if it ever worked.

Definitely, I never had that problem in the past.

Flags: needinfo?(spamcop)

I can confirm the issue is not reproducible with the current release version 66.0.2, so it was introduced somewhere between 66.0.2 and 67.0b6 (and persists up to 67.0b8).

I also can't reproduce. Tried with nightly and "Form History Control (II)" which is in the screencast, on mac OS. We're going to need a regression window or better steps to reproduce to do anything here.

(In reply to TGOS from comment #4)

(In reply to (behind on needinfos) Jared Wein [:jaws] (please needinfo? me) from comment #3)

TGOS, can you run mozregression to find when this behavior started happening?

I cannot install mozregression, when attempting, I get:

Cannot uninstall 'six'. It is a distutils installed project and thus we cannot accurately determine which files belong to it which would lead to only a partial uninstall.

How are you installing mozregression? Sticking the error into google yields:

https://github.com/pypa/pip/issues/3165#issuecomment-146666737

which suggests you can work around it with those flags if using pip to install it.

Flags: needinfo?(spamcop)

(In reply to :Gijs (he/him) from comment #6)

How are you installing mozregression?

The page only lists a single way how to install it, so I don't quite understand the question.

Using "--ignore-installed six" works but I don't know what values to use for good and for bad. I only know that everything works fine with the 66 Release and it's broken in 67.0b6. I've never used any nightly.

When I look up the build ID of 66 (20190326175229) and the one of 67.0b8 (20190404130536), then I get only two good builds, then it terminates, stating the 2nd one should be bad.

Flags: needinfo?(spamcop)

(In reply to TGOS from comment #7)

(In reply to :Gijs (he/him) from comment #6)

How are you installing mozregression?

The page only lists a single way how to install it, so I don't quite understand the question.

Using "--ignore-installed six" works but I don't know what values to use for good and for bad. I only know that everything works fine with the 66 Release and it's broken in 67.0b6. I've never used any nightly.

When I look up the build ID of 66 (20190326175229) and the one of 67.0b8 (20190404130536), then I get only two good builds, then it terminates, stating the 2nd one should be bad.

--good 66 would normally work, it'll automatically pick the point at which 66 branched from nightly. Don't put anything for 'bad', and it'll use today's nightly build.

If you can't reproduce the issue with a clean profile on nightly, there's likely something else about your profile that's necessary to cause this issue. Can you attach a copy of about:support from the regular profile where you're seeing this, and/or try reproducing with a clean profile against nightly? (just running nightly should create its own profile these days, though otherwise see https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles ).

Flags: needinfo?(spamcop)

(In reply to :Gijs (he/him) from comment #8)

If you can't reproduce the issue with a clean profile on nightly, there's likely something else about your profile that's necessary to cause this issue.

I'm not using a clean profile, I make it clone my existing profile with all add-ons and settings using the --profile option for every test.

How should I reproduce it with a clean profile? I would have no More Tools menu at all in the title bar and no add on I can open there to get an input field I can type into. It's impossible to reproduce without first installing an add-on every time, customizing the toolbar and moving this add-on to the more tools menu.

But I cannot reproduce it with my profile in nightly builds, just like I cannot reproduce it with my profile in release version 66. I can only reproduce it in 67 Beta builds. If you tell me where I can download the builds 67b1 to 67b6, then I can tell you which the first beta build showing that issue, maybe it was still working in 67b1, I cannot say for sure when it broke, nor why it doesn't happen in nightly builds.

Flags: needinfo?(spamcop)

(In reply to TGOS from comment #9)

(In reply to :Gijs (he/him) from comment #8)

If you can't reproduce the issue with a clean profile on nightly, there's likely something else about your profile that's necessary to cause this issue.

I'm not using a clean profile, I make it clone my existing profile with all add-ons and settings using the --profile option for every test.

How should I reproduce it with a clean profile? I would have no More Tools menu at all in the title bar and no add on I can open there to get an input field I can type into. It's impossible to reproduce without first installing an add-on every time, customizing the toolbar and moving this add-on to the more tools menu.

But I cannot reproduce it with my profile in nightly builds, just like I cannot reproduce it with my profile in release version 66. I can only reproduce it in 67 Beta builds. If you tell me where I can download the builds 67b1 to 67b6, then I can tell you which the first beta build showing that issue, maybe it was still working in 67b1, I cannot say for sure when it broke, nor why it doesn't happen in nightly builds.

Hm, interesting. Sounds like we broke it and then fixed it? You could try doing --bad 67 and seeing if you see the bug on that build, but it's also possible we broke something on 68 and then uplifted it to 67, but haven't yet uplifted whatever fixed it again on 68 to 67... You could exclude that by checking earlier betas like you said, I guess...

If you want to manually check betas, you can find them at https://ftp.mozilla.org/pub/firefox/releases/, which lists 67.0b3-8. I assume you've tried updating your local beta and checked it's still broken on b8?

Flags: needinfo?(spamcop)

I installed 67b3, there is no earlier 67 version available. It doesn't want to use my existing profile, so it came up with a clean profile. I installed form history control, I moved it to the More Tools menu and I could immediately reproduce the issue directly on the addons.mozilla.org page: just clicked into the search field, then opened form history control, focused the search field, typed a letter and it ended up in the search field of the web page. Please note, that this was a clean profile, all settings on default values and all I did was installing this one add-on.

Flags: needinfo?(spamcop)

[Tracking Requested - why for this release]:
Usability regression with add-ons in the overflow panel.

OK, so:

8cd5c2339ca1	Henri Sivonen — Bug 1535541 Use chrome-process TabParent focus instead of APZ focus for keyboard event routing. r=nika
2244c803a5d0	Henri Sivonen — Bug 1524242 - Capture TabParent of out-of-process iframe when creating TextComposition. r=masayuki
aa59ec8e0542	Henri Sivonen — Bug 1524976 - Lookup TabParent by TabParent::GetFocused() in EventStateManager::DoContentCommandEvent(). r=masayuki
dbe5292cac33	Henri Sivonen — Bug 1535537 - Maintain information about focused TabParent on chrome main thread independently of APZ. r=nika

These "fixed" some of this on nightly, in that now mouse focus was completely wonky (have to click twice to focus the input, after the first click focus is in limbo), but then after bug 1536730 this was fixed "properly" on nightly for me.

I'm still looking at what regressed this. m-c window was:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=996a48b306521112d483b3bd8bede1e4fa23d3fa&tochange=4ab143dde4dc3424cfedc74b3648fbf2e47fb7bf

my current prime suspect is bug 1466208.

Component: Toolbars and Customization → User events and focus handling
Depends on: 1536730
Keywords: regression
Product: Firefox → Core

OK, so this is why I shouldn't take bets on regressors until I have an inbound window. Actual regressing changes (un-CC'ing Masayuki):

Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0fd1ec8af3fd1e2a75f08613414ed590e76ea286&tochange=e5162aed5ec0228cf24af6c22fa03c5e1963aea9

e5162aed5ec0	Henri Sivonen — Bug 1524231 - Find TabParent to dispatch to by LayersId attached to the event. r=nika
273f979acb06	Henri Sivonen — Bug 1532334 - Make nsContentUtils::IsSubDocumentTabbable() return true for out-of-process iframes. r=nika
627cd6785c62	Henri Sivonen — Bug 1524980 - Use RemoteFrameChild if present in nsFocusManager::Focus(). r=nika
ed8a0908ddf0	Henri Sivonen — Bug 1524977 - Use RemoteFrameChild if present in nsFocusManager::GetNextTabbableContent(). r=nika

Henri, can you please take a look? Perhaps we can just back these patches out of beta given that we don't ship fission with 67? Or is the fix in bug 1536730 upliftable without any of the other stuff?

For the avoidance of doubt, STR:

  1. clean profile on mac (haven't tested windows/linux though I expect the same issue there)
  2. install https://addons.mozilla.org/addon/form-history-control/
  3. right click add-on icon, click 'pin to overflow menu'
  4. focus the search box in addons.mozilla.org
  5. type something recognizable, e.g. "foo"
  6. click overflow button, then click add-on button
  7. click into the search input field on the top right of the add-on's frame in the overflow panel (note: you get a blinking cursor so focus worked)
  8. mash random keys on keyboard

ER:
keyboard input into search field in the panel

AR:
no change to the panel's search field, but if you dismiss the panel your keyboard mashing has gone to the field on addons.mozilla.org instead.

Note: this does not reproduce in nightly, see comment #12, this regression only affects 67 beta now that other changes have landed on 68.

Flags: needinfo?(hsivonen)
No longer depends on: 1536730
Flags: needinfo?(hsivonen)
Regressed by: 1536730
Regressed by: 1524231
No longer regressed by: 1536730

This changeset reverts:

  • Bug 1524231 - Find TabParent to dispatch to by LayersId attached to the event.
  • Bug 1532334 - Make nsContentUtils::IsSubDocumentTabbable() return true for out-of-process iframes.
  • Bug 1524980 - Use RemoteFrameChild if present in nsFocusManager::Focus().
  • Bug 1524977 - Use RemoteFrameChild if present in nsFocusManager::GetNextTabbableContent().
  • Bug 1534258 - Send Deactivate() messages to out-of-process iframes.
  • Bug 1534255 - Enable out-of-process iframes to take APZ focus.

Comment on attachment 9056864 [details]
Bug 1541088 - Revert focus-related Fission event delivery changes on beta.

Beta/Release Uplift Approval Request

  • Feature/Bug causing the regression: Bug 1524231
  • User impact if declined: Keyboard events sometimes don't go to the right place in Web Extension-created UI.

(Unclear which bug exactly in the regression range caused the regression, providing the bug number for one of them.)

  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 13
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This change is relatively low-risk, because it reverts patches, so we are going back to a state we already were in before. Alternatives are: 1) finding the root cause precisely and creating a more targeted change and 2) Uplifting later Fission event delivery changes that fixed this bug on Nightly.
  • String changes made/needed: None
Attachment #9056864 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]
Flags: qe-verify? → qe-verify+

Comment on attachment 9056864 [details]
Bug 1541088 - Revert focus-related Fission event delivery changes on beta.

Let's backout the regressing patches as they are not needed for 67, uplift accepted for 67 beta 10, thanks.

Attachment #9056864 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Reproduced this issue on Mac OS 10.14 using the str provided in comment 13, with 67.0b6.

Confirming the fix is no longer reproducing on latest Nightly, build ID 20190410215612.

Leaving the qe+ flag in place until verification on beta is also performed.

Marking this as resolved since the fix arrived on beta, too.

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED

Confirming the fix on macOS 10.14.3 using Fx 67.0b10, buildID 20190411084603.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Thanks for reporting and thanks for confirming and sorry about the regression.

Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: