1486984 - regression: searching pdfs is not possible

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect, P1)

Description

[Djfe]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20180828220157

Steps to reproduce:

I opened https://dlcdnets.asus.com/pub/ASUS/mb/LGA1151/MAXIMUS_VIII_RANGER/G10485_MAXIMUS_VIII_RANGER_UM_WEB.pdf


Actual results:

The search bar doesn't come up when pressing STRG+F

Menubar->Edit->Search page is also greyed out


Expected results:

This seems to be a regression since the search bar works fine on release.

Comment 1

[Masatoshi Kimura [:emk]]

Could you find a regression range using mozregression?

Comment 2

[Rares Doghi, Desktop QA]

Hi,

I tried reproducing this issue using the URL in the description but I was unable to access the link, I also tried different pdf files like this one http://lynxpride.org/docs/7/6/10%20Rules%20For%20Success%20ET%20-%20Google%20Docs.pdf and the Search worked using the latest version of Firefox Nightly 63.0a1 (2018-09-02) as well as with older versions of Firefox Nightly.

I did however managed to reproduce this issue if the PDF file is opened in browser and then the browser is updated to the latest version. After the browser is restarted and the PDF file is loaded the user is unable to use the search to find anything in page, also the "Find in this page" option is greyed out in the menu bar.

Please note that we cant find a regression range for this issue since we cant update the Browser to a specific version.

I used an older version of Firefox Beta, updated to the latest Beta and the issue does not occur.
I also used Release 60.0 and updated to 60.0.2 and the issue does not occur there either, it seems only upgrading to the latest Nightly will trigger this issue.

Comment 3

[Pascal Chevrel:pascalc]

Mozilla/5.0 (X11; Linux x86_64; rv:64.0) Gecko/20100101 Firefox/64.0 ID:20180904220134

With the pdf in comment #0 I indeed had difficulties getting the search bar to pop up with ctrl+F (had to do it like 5 times) while it worked in another smaller pdf. I wonder if the size of the pdf document matters.

Comment 4

[David Durst [:ddurst]]

Can you test again with varying size PDFs, please?

Comment 5

[B.J. Herbison]

While testing the fix for bug 1062025 I ran into this issue.

- With https://www.irs.gov/pub/irs-pdf/fw4.pdf (four pages long) I wasn't able to get control-F to respond after a restart until I refreshed the tab.

- With https://www.fidelity.com/bin-public/060_www_fidelity_com/documents/automatic-withdrawals-ira.pdf (six pages) even refreshing the tab didn't usually allow control-F to work, although on one attempt I got a find bar to appear but control-F wouldn't focus on it. (I haven't been able to reproduce that situation.)

I'm using the ASan Nightly on Linux.

Comment 6

[Rares Doghi, Desktop QA]

Hi, I retested this issue using the above mentioned steps from Comment 2 and it still occurs using this PDF: 
https://www.istqb.org/downloads/send/51-ctfl2018/208-ctfl-2018-syllabus.html

I did however notice that it no longer occurs with smaller PDF files, like the ones from Comment 5.
I tested this issue on Windows 10 as well as Ubuntu 18.04.

Comment 7

[[:philipp]]

a user on sumo described a similar sounding issue with reproducible steps:
- Open a pdf in Firefox (i used http://unec.edu.az/application/uploads/2014/12/pdf-sample.pdf)
-Ctrl-F and search something (not required to hit any result)
-Close the search box
-Switch to another tab, do some scrolling (i used https://www.google.com/search?q=test)
-Switch back
-Ctrl-F doesn't call out the search box
https://support.mozilla.org/en-US/questions/1235335

running mozregression with these steps points towards bug 1482648 as having introduced the issue.

Comment 8

[Jim Mathies [:jimm]]

I can reproduce, win7 nightly 64.

Comment 9

[David Durst [:ddurst]]

Who owns this these days?

Comment 10

[Brendan Dahl [:bdahl]]

I still triage, but I'm hoping Paolo can fix the regression. I pinged him yesterday but it was a bit late his time. I'll try again.

Comment 11

[:Paolo Amadini]

Yes, I can look into this.

Comment 12

[:Paolo Amadini]

Attachment: Bug 1486984 - Fix find commands for PDF and special pages, and remove obsolete code. r=Gijs

This restores the intended behavior, that didn't work as expected in multi-process mode.

Comment 13

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=331adae760f5b418ff1f4ac68aa70714f3a82dc8

Comment 14

[:Paolo Amadini]

This is another example of the advantages of removing levels of indirection through "observes" and "command". The original code was broken because it inadvertently controlled the same elements in two different places, but worked by chance because of the unspecified behavior of nested "observes" and "command", and the fact that some code was not executed in e10s mode. Bug 1482648 surfaced the issue.

Comment 15

[Pascal Chevrel:pascalc]

Paolo, do you think you can land a final patch and request an uplift in the process today?

Comment 16

[:Paolo Amadini]

(In reply to Pascal Chevrel:pascalc from comment #15)
> Paolo, do you think you can land a final patch and request an uplift in the
> process today?

Not sure, this probably needs to be verified in mozilla-central first. Anyways, I'll add Gijs as a reviewer in case he has time.

Comment 17

[:Paolo Amadini]

For Beta, we may also consider backing out revision 981a904be103, that fixed the issue for me.

Comment 18

[:Gijs (out for now; he/him)]

I guess this might fix bug 967517 and/or bug 1094240 as well?

Comment 19

[:Paolo Amadini]

Quite likely!

Comment 20

[:Paolo Amadini]

(In reply to :Paolo Amadini from comment #17)
> For Beta, we may also consider backing out revision 981a904be103, that fixed
> the issue for me.

I filed bug 1497926 for the backout, as I think it's safer anyways, and for this patch I have to do some testing to address an improvement suggested in the review.

Comment 21

[[:philipp]]

the steps from comment #7 no longer reproduce in 63.014 after the backout in bug 1497926

Comment 22

[u626267]

I use Firefox 64.0a1 (2018-10-13) and I cannot reproduce the issue anymore. I think Firefox 64 is no longer affected.

Comment 23

[[:philipp]]

(In reply to Benedikt Neumayr from comment #22)
> I use Firefox 64.0a1 (2018-10-13) and I cannot reproduce the issue anymore.
> I think Firefox 64 is no longer affected.

hi benedikt, i can still reproduce the steps from comment #7 in the current nightly.
so far action on this issue has only been taken on 63.0b (the change regressing this has been backed out from the beta channel), so it's expected that the problem is still present in 64.0a1.

Comment 24

[:Paolo Amadini]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e60f28a18b22e751d1d1d8d5ce2ccd002ef5601

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7327a893e2a2
Fix find commands for PDF and special pages, and remove obsolete code. r=Gijs

Comment 26

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/7327a893e2a2

Comment 27

[Rares Doghi, Desktop QA]

Hi guys, I can no longer reproduce this issue on Nightly 65.0a1 (2018-10-31 or Firefox Release 63.0.1, but following the steps from Comment 7 I can Still reproduce the issue in Firefox Beta 64.0b5.

Comment 28

[:Paolo Amadini]

Comment on attachment 9015270 [details]
Bug 1486984 - Fix find commands for PDF and special pages, and remove obsolete code. r=Gijs

Not verified by QA in Nightly yet.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1482648

User impact if declined: The "Find" command is inconsistently enabled or disabled

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: Check that the "Find" command is disabled in these cases:
 - Image
 - about:preferences
 - about:addons
 - about:config

Check that the "Find" command is enabled in these cases:
 - about:blank and other about pages
 - Text file
 - HTML page
 - PDF viewer

Check that there are no regressions with the "View Source" command, in other words that is still enabled when it was before in the cases above.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Risk is limited to the feature, the fix was in Nightly for one week and was confirmed, it is still early in the Beta cycle

String changes made/needed: None

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9015270 [details]
Bug 1486984 - Fix find commands for PDF and special pages, and remove obsolete code. r=Gijs

[Triage Comment]
Fixes broken Find in the PDF viewer. Approved for 64.0b6.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a15c1d469c75

Comment 31

[Rares Doghi, Desktop QA]

Hi, I have been following the steps from Comment 7 and the issue no longer occurs in Firefox Beta 64.0b6 (20181101155334), Release 63.0.1 (20181030165643) or Nightly 65.0a1 (2018-11-01).

However, On Release 63.0.1 these issues from Comment 28 still occur:

Image - Ctrl + F and Edit > Find In This Page... can both be used
about:Addons - Ctrl + F and Edit > Find In This Page... can both be used
about:Config - Ctrl + F does not work but Edit > Find In This Page... does work
about:Preferences - Ctrl + F does not work but Edit > Find In This Page... does work

Should I mark it as Verified for Release as well just because the issue from Comment 7 no longer occurs? or we should wait for a fix for the enabled Find in this page.. option in those 4 pages ?

In the meantime I will mark this issue accordingly.

Comment 32

[:Paolo Amadini]

(In reply to Rares Doghi from comment #31)
> Image - Ctrl + F and Edit > Find In This Page... can both be used
> about:Addons - Ctrl + F and Edit > Find In This Page... can both be used
> about:Config - Ctrl + F does not work but Edit > Find In This Page... does
> work
> about:Preferences - Ctrl + F does not work but Edit > Find In This Page...
> does work
> 
> Should I mark it as Verified for Release as well just because the issue from
> Comment 7 no longer occurs? or we should wait for a fix for the enabled Find
> in this page.. option in those 4 pages ?

By looking at the code I believe that the behavior on Release for these cases is not the intended one, so I'd say this bug has also fixed these instances. Thanks for checking!

Comment 33

[Rares Doghi, Desktop QA]

(In reply to :Paolo Amadini from comment #32)

> By looking at the code I believe that the behavior on Release for these
> cases is not the intended one, so I'd say this bug has also fixed these
> instances. Thanks for checking!

Does this mean there will be a Fix Uplifted for Release ? should we set the flags for Fx63 back to "Affected" ?

Comment 34

[:Paolo Amadini]

Sorry, my comment referred to the original behavior in Release 62 and previous versions, that is also the behavior in Release 63 since the regressing bug was backed out in bug 1497926. I'll mark the status for 63 as unaffected, feel free to update if that's not the correct one.

Comment 35

[Rares Doghi, Desktop QA]

Thanks Paolo, Based on Comment 34 I will mark this issue accordingly.