Closed Bug 1439974 Opened 4 years ago Closed 4 years ago

"Quick Find within link-text only" now requires pressing [enter] twice to visit link

Categories

(Toolkit :: Find Toolbar, defect)

60 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- unaffected
firefox59 --- unaffected
firefox60 + verified
firefox61 --- verified

People

(Reporter: groovecoder, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(1 file)

This seems to be a regression (or intentional change of behavior?) on build 60.0a1 (2018-02-20) (64-bit) 

I'm on Mac OS 10.13.3

Steps to Reproduce:
1. Open the "Quick Find within link-text only" feature (my keyboard shortcut is the default ' key)
2. Type some text to find links
3. Press [enter]

Expected results:
Pressing [enter] should visit the link

Actual results:
Pressing [enter] closes the quickfind toolbar, but does not visit the link. I have to press [enter] twice to close the quickfind toolbar, and then visit the link.
[Tracking Requested - why for this release]: Regression.
Adding Boris based on the regression window in comment 2.
Flags: needinfo?(bzbarsky)
So the problem is that the code in Finder.jsm is not actually working with a key event.  It's working with a faked-up version created at https://searchfox.org/mozilla-central/rev/9a8d3f8191b15cdca89a7ce044c7bea2dd0462dc/toolkit/modules/RemoteFinder.jsm#316-318 which comes from an equally faked-up thing at https://searchfox.org/mozilla-central/rev/9a8d3f8191b15cdca89a7ce044c7bea2dd0462dc/toolkit/modules/RemoteFinder.jsm#191-198

Duck-typing strikes again, when the duck turns out to actually be a mongoose with glued-on feathers.
Assignee: nobody → bzbarsky
I'd love ideas on how to test this
Attachment #8953183 - Flags: review?(felipc)
Attachment #8953183 - Flags: review?(felipc) → review+
Maybe Mike has ideas on how to test it?
Flags: needinfo?(mdeboer)
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c351e7dca893
Pass actual keypress events to the finder's keypress method, even in the e10s case.  r=felipe
https://hg.mozilla.org/mozilla-central/rev/c351e7dca893
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Flags: needinfo?(bzbarsky)
The fix only seems to work for me for the first link navigation in a given outer window, later ones throw "TypeError: can't access dead object" on the line:

        this._finder.keyPress(new this.KeyboardEvent("keypress", data));
Ugh.  I didn't realize we attached this thing to the outer window, not the inner.

Filed bug 1441241 and will fix there.
And Simon, thank you for catching that!
Well, since it's a bug specific to RemoteFinder.jsm, I think that adding a test to or a new test similar to https://searchfox.org/mozilla-central/source/toolkit/modules/tests/browser/browser_Finder.js would work.
I'm not sure how useful a regression test for this specific fix might be, but that's up to Boris. Thanks for fixing this and the follow-up!
Flags: needinfo?(mdeboer)
I managed to reproduce the bug using an older version of  Nightly (2018-02-21) on Windows 10 x64.
I retested everything on latest Nightly 61.0a1 and beta 60.0b4 using Windows 10 x64, macOS 10.13 and Ubuntu 16.04 and the bug is not reproducing anymore. When you hit enter for the first time, the selected link is opened.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.