Closed Bug 1397989 Opened 2 years ago Closed 2 years ago

Find toolbar sometimes failing to show

Categories

(Toolkit :: Find Toolbar, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla58
Tracking Status
firefox57 --- verified
firefox58 --- verified

People

(Reporter: dcamp, Assigned: mikedeboer)

References

Details

Attachments

(1 file)

Using ctrl+F on windows 10, sometimes the find toolbar stops showing up.  This has happened twice to me today.  In both cases it worked in different tabs.
Dave, does this mean it doesn't show up on certain websites? Some pages capture/ override the Ctrl+f shortcut, which may result in this behavior...
Flags: needinfo?(dcamp)
No, this has been happening on websites that I use find on all the time, and if I switch tabs and load the same site it will start working.
Flags: needinfo?(dcamp)
Just happened again - after switching tabs coming back, the search bar started working again.
Without a good STR this is very hard to fix, because I've never experienced this myself.
Priority: -- → P3
Duplicate of this bug: 1400029
Duplicate of this bug: 1405779
OK, I finally hit it myself and can consistently reproduce it:

1. Copy this URL to your clipboard: https://public.etherpad-mozilla.org/p/mozanime
2. Open Nightly (local build for me) and have it finish loading about:home
3. Paste the URL from 1) in the urlbar and hit Enter and wait 'till the page finished loading
4. Try to open the findbar using CMD/CTRL+f

It won't.

The cause:
 - browser-child.js sends a statechange up to RemoteWebProgress.jsm that contains
   a `documentContentType` value of `null`, along with `requestURI` and `originalRequestURI`
   _after_ other state changes that did send a valid content-type.
 - The content-type is used by the WebProgressListener in browser.js to toggle the
   disabled state of the 'isImage' broadcaster.
 - The 'isImage' broadcaster is used by the 'cmd_find' and 'cmd_findAgain' commands to
   determine whether they should be enabled. In this case: not.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Priority: P3 → P1
Comment on attachment 8917033 [details]
Bug 1397989 - Make sure to only set non-empty values as the content-type of a document in the remote browser binding.

https://reviewboard.mozilla.org/r/188052/#review193210

Nice find! Thanks!

::: commit-message-67fc6:1
(Diff revision 1)
> +Bug 1397989 - Make sure to only set non-empty values as the content-type of a document in a browser binding. r?mconley

Nit: "in a browser binding" -> "in the remote browser binding".
Attachment #8917033 - Flags: review?(mconley) → review+
Thanks for the mega-quick review!
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c86b1f93b84
Make sure to only set non-empty values as the content-type of a document in the remote browser binding. r=mconley
https://hg.mozilla.org/mozilla-central/rev/4c86b1f93b84
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Comment on attachment 8917033 [details]
Bug 1397989 - Make sure to only set non-empty values as the content-type of a document in the remote browser binding.

Approval Request Comment
[Feature/Bug causing the regression]: Not entirely sure, but somewhere up the road empty content-types could be sent up from the content process.
[User impact if declined]: 'Find (Again)' and 'View Source' would stay disabled even though the document loaded normally.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: Yes, STR can be found in comment 7.
[List of other uplifts needed for the feature/fix]: n/a.
[Is the change risky?]: Minor risk.
[Why is the change risky/not risky?]: Because a content-type of `null` is not something the front-end code is built to deal with as a document end state.
[String changes made/needed]: n/a.

I'd like to see this considered for uplift, because Dave reported this a month ago already and I was able to reproduce it just yesterday. But the UX is rather jarring and makes the user feel the browser is unreliable/ flaky.
Attachment #8917033 - Flags: approval-mozilla-beta?
Comment on attachment 8917033 [details]
Bug 1397989 - Make sure to only set non-empty values as the content-type of a document in the remote browser binding.

Seems like a must fix, beta57+
Attachment #8917033 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
See Also: → 1396796
Duplicate of this bug: 1404676
Verified fixed using the latest Nightly  58.0a1(2017-10-16), following the STR from comment 7, on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
I reproduced this issue using Fx 57.0a1, build ID: 20170907100318, on Windows 10 x64, following the STR from comment 7.
I can confirm this issue is fixed, I verified using Fx 57.0b9, on Windows 10 x64, Ubuntu 14.04 LTS and mac OS X 10.12.6.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Duplicate of this bug: 1394095
You need to log in before you can comment on or make changes to this bug.