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

Status: VERIFIED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Luke Crouch [:groovecoder]]

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.

Comment 1

[Kevin Brosnan [Ex-Mozilla]]

[Tracking Requested - why for this release]: Regression.

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9848ae936a3d35820e1e9a6b69eeb856aa53aa78&tochange=55d20d1f137a5fc807860e283354ce6e0ad8d010

Suspect: Bug 1436508

Comment 3

[Julien Cristau [:jcristau]]

Adding Boris based on the regression window in comment 2.

Comment 4

[Boris Zbarsky [: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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Pass actualy keypress events to the finder's keypress method, even in the e10s case

I'd love ideas on how to test this

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Maybe Mike has ideas on how to test it?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
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

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c351e7dca893

Comment 9

[Simon Lindholm]

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

Comment 10

[Boris Zbarsky [:bzbarsky]]

Ugh.  I didn't realize we attached this thing to the outer window, not the inner.

Filed bug 1441241 and will fix there.

Comment 11

[Boris Zbarsky [:bzbarsky]]

And Simon, thank you for catching that!

Comment 12

[Mike de Boer [:mikedeboer]]

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!

Comment 13

[Oana Botisan, Desktop QA]

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.