Find in page does not display results on screen when page is zoomed

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: ashah, Assigned: mbrubeck)

Tracking

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

8 years ago
Build:
Mozilla/5.0(Android;Linux armv7l;rv2.0b7pre)Gecko/20100916/4.0b7pre Fennec/2.0b1pre

STR:
1. Open www.mozilla.com
2. Zoom in on the page.
3. Open the find bar and search for the keyword "fire"
4. Press the up/down arrow on the find bar in order to see the results

Expected result:
The word should appear on the screen in front of the user

Actual result: 
The results are hidden behind the find bar (i.e. it is not shown to the user on the screen in front of him). This might lead him to think that there are no more find results(more prominent when the page is zoomed).

FWIW, move the page in order to show the results to the user on the screen in front of him. The results should not be hidden behind the find bar
(Reporter)

Comment 1

8 years ago
Also seeing this without the zoom. specially when you search for common words like "the","is","of" etc
The "zoom and pan to an element" is broken for some cases. Watch out for dupes! The same code is used for Form Assistant.
(Reporter)

Updated

8 years ago
tracking-fennec: --- → ?
Also occurs on N900 : 

verified FIXED on build:
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20100916 Firefox/4.0b7pre Fennec/2.0b1pre
OS: Android → All
The verified FIXED on build should not be noted in the last comment please ignore that portion.

The issue still occurs on the N900:
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20100916 Firefox/4.0b7pre
Fennec/2.0b1pre

Updated

8 years ago
tracking-fennec: ? → 2.0b2+
This seems to be working fine for me on today's Android build. I tested the "www.mozilla.com" site for "fire", like in comment 0.

I saw all the found "fire" spots. I compared with desktop firefox to make sure fennec found all the same occurrences.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
Issue still occurs on Maemo :
Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20100927 Firefox/4.0b7pre Fennec/2.0b1pre

Issue does not occur on Android:
Mozilla/5.0 (Android; Linux armv71; rv2.0b7pre) Gecko/20100927 Firefox/4.0b7pre Fennec/2.0b1pre

Changed Platform from All to Maemo to match.
Status: RESOLVED → REOPENED
OS: All → Maemo
Resolution: WORKSFORME → ---
(In reply to comment #6)
> Issue still occurs on Maemo :
> Mozilla/5.0 (Maemo;Linux armv71; rv:2.0b7pre)Gecko/20100927 Firefox/4.0b7pre
> Fennec/2.0b1pre
> 
> Issue does not occur on Android:
> Mozilla/5.0 (Android; Linux armv71; rv2.0b7pre) Gecko/20100927 Firefox/4.0b7pre
> Fennec/2.0b1pre
> 
> Changed Platform from All to Maemo to match.

Naoki,
It works for me on Maemo using the steps to reproduce descrivbe in comment 0 and the latest nightly.
Do you have another steps to repro?

Comment 8

8 years ago
I can reproduce this with:

Device: Nokia N900
BuildID: Mozilla /5.0 (Maemo;Linux armv7l;rv:2.0b8pre)Gecko/20101021
Firefox/4.0b8pre Fennec /4.0b2pre 

Device: HTC Desire A8181
BuildID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b8pre) Gecko/20101021
Firefox/4.0b8pre Fennec /4.0b2pre  

Steps to reproduce:

1. Visit www.mozilla.org
2. Bring up the “Find In Page” toolbar by tapping on the page favicon and then “Find in page”.
3. Enter string “mozilla” in the search field. 
4. Use arrow-buttons to navigate through your results 

On HTC Desire, I see ghost tab thumbnails when navigating from one match to another. Bug 604946 was logged for this but with different steps to reproduce and is Verified Fixed.
Created attachment 485259 [details] [diff] [review]
Patch

Naoki has tried to help me reproduce it on IRC but for some reasons I can't :(
 
What I suspect here is we are calculating the zoom level on a wrong height with results to misplaced the element to looks like they are above the find helper (even if they can't _really_ be above the content-navigator-helper element like it was the cas 1 year ago)

Fixing the zoom to calculate on the correct height seems a good solution here (and a must do) but I can not really affirm it will fix the bug since I can reproduce it.
Attachment #485259 - Flags: review?(mbrubeck)
Attachment #485259 - Flags: review?(mark.finkle)
(In reply to comment #8)
> I can reproduce this with:
> 
> Device: Nokia N900
> BuildID: Mozilla /5.0 (Maemo;Linux armv7l;rv:2.0b8pre)Gecko/20101021
> Firefox/4.0b8pre Fennec /4.0b2pre 
> 
> Device: HTC Desire A8181
> BuildID: Mozilla /5.0 (Android;Linux armv7l;rv:2.0b8pre) Gecko/20101021
> Firefox/4.0b8pre Fennec /4.0b2pre  
> 
> Steps to reproduce:
> 
> 1. Visit www.mozilla.org
> 2. Bring up the “Find In Page” toolbar by tapping on the page favicon and then
> “Find in page”.
> 3. Enter string “mozilla” in the search field. 
> 4. Use arrow-buttons to navigate through your results 
> 
> On HTC Desire, I see ghost tab thumbnails when navigating from one match to
> another. Bug 604946 was logged for this but with different steps to reproduce
> and is Verified Fixed.

Sorry I've posted my comment without seeing yours. Thanks for sharing your steps to reproduce.

Using your steps, can you explain the behavior you're seeing? Basically my question is: do you have the feeling to have the find result _under_ the find bar or does the view point to something completely unrelated?

Thanks.

Comment 11

8 years ago
Matched strings are highlighted for a second then the focus jumps somewhere else in the page (see http://www.facebook.com/video/?id=100001755601293#!/video/video.php?v=100319606703226)
(In reply to comment #11)
> Matched strings are highlighted for a second then the focus jumps somewhere
> else in the page (see
> http://www.facebook.com/video/?id=100001755601293#!/video/video.php?v=100319606703226)

Since I don't have a facebook account I can't see the video but hopefully your comment has made me thought of it with a different approach and I've found the that we receive a "scroll" event from the content _after_ the zoom is done. This scroll event was forcing us to display the wrong view.

Thanks for your participation!
Created attachment 485296 [details] [diff] [review]
Patch v0.2

The patch just call the asyncSendMessage outside the FindAssist:Find message to let the scroll event propagate _before_ we're zooming to the right position.
The results is that we can see two zoom actions, the first because Fennec react to the "scroll" event and the second because Fennec react to the "FindAssist:Show" event. 

So the plan is to disabled the "scroll" event on the browser while the Find helper is opened.
Attachment #485259 - Attachment is obsolete: true
Attachment #485296 - Flags: review?(mark.finkle)
Attachment #485259 - Flags: review?(mbrubeck)
Attachment #485259 - Flags: review?(mark.finkle)
Comment on attachment 485296 [details] [diff] [review]
Patch v0.2

I don't like turning scroll syncing off like this:

>+    browser.messageManager.removeMessageListener("scroll", browser._messageListenerRemote);

>+    browser.messageManager.addMessageListener("scroll", browser._messageListenerRemote);

Do we need to use this since you are using a timeout to send the FormAssist:Show?

Can we add a property to the browser to disable scrollSync? browser.scrollSync = true | false  ?
(In reply to comment #14)
> Comment on attachment 485296 [details] [diff] [review]
> Patch v0.2
> 
> I don't like turning scroll syncing off like this:
> 
> >+    browser.messageManager.removeMessageListener("scroll", browser._messageListenerRemote);
> 
> >+    browser.messageManager.addMessageListener("scroll", browser._messageListenerRemote);
> 
> Do we need to use this since you are using a timeout to send the
> FormAssist:Show?
 
Yes, because the timeout ensure the "scroll" event arrive before the FindAssist:Show message but we still need to ignore it to ensure the view is not scrolled twice.
Created attachment 485684 [details] [diff] [review]
Patch v0.3
Attachment #485296 - Attachment is obsolete: true
Attachment #485684 - Flags: review?(mark.finkle)
Attachment #485296 - Flags: review?(mark.finkle)
Comment on attachment 485684 [details] [diff] [review]
Patch v0.3

Maybe in the future we can somehow move more of this logic into the browser binding, so we don't need to expose the "scrollSync" property. For now, this is better.
Attachment #485684 - Flags: review?(mark.finkle) → review+
Does this not affect FormHelper? I guess the find system is controlling the scrolling, which is different than FormHelper.
http://hg.mozilla.org/mobile-browser/rev/8be7b23e97cb
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 20

8 years ago
Created attachment 485966 [details] [diff] [review]
followup

The fix had an error; "this.scrollSync" always returns undefined.  This regressed bug 598391.  Here's the one-line fix.
Assignee: 21 → mbrubeck
Status: RESOLVED → REOPENED
Attachment #485966 - Flags: review?(mark.finkle)
Resolution: FIXED → ---
Attachment #485966 - Flags: review?(mark.finkle) → review+
pushed followup:
http://hg.mozilla.org/mobile-browser/rev/b064cb6eadf7
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
verified FIXED on builds:


Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b8pre) Gecko/20101026 Namoroka/4.0b8pre Fennec/4.0b2pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101026 Namoroka/4.0b8pre Fennec/4.0b2pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.