Closed Bug 1306234 Opened 3 years ago Closed 3 years ago

Dimmed background is inconsistent after clicking "Highlight All"

Categories

(Toolkit :: Find Toolbar, defect)

52 Branch
defect
Not set
Points:
2

Tracking

()

VERIFIED FIXED
mozilla52
Iteration:
52.2 - Oct 17
Tracking Status
firefox49 --- unaffected
firefox50 --- disabled
firefox51 --- disabled
firefox52 --- verified

People

(Reporter: simona.marcu, Assigned: mikedeboer)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Attached video reversedBackground.mp4
Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20160928030201

[Affected versions]:
- Nightly 52

[Affected platforms]:
- Windows 10, Ubuntu 14.04, Mac OS X 10.11

[Steps to reproduce]:
1. Launch Firefox
2. Log into https://bugzilla.mozilla.org/userprefs.cgi?tab=settings
3. Select "Bugzilla's general appearance (skin)" to "Classic"  in "User Interface" section and Submit changes
4. Open the Find Bar
5. Search for "bugzilla"
6. Click twice on "Highlight All" button
  
[Expected result]:
In steps 5 and 6, the dimmed background should be the same.

[Actual result]:
Inconsistent dimmed background, after step 6, the dimmed background is reversed. Please see the attached scree-cast for more details.
This is also reproducable on https://developer.mozilla.org/en-US/, so I'll take that as an easier to reproduce example.

This is what happens on https://developer.mozilla.org/en-US/:
1. 'm' has about 80 matches, most of them are in the part of the page that has a light background, thus the black dim is shown.
2. 'mo' has about 20 matches, the same scenario as 1.
3. 'moz' has about 10 matches, most of them are in the part of the page that has a dark background, thus the white dim is shown.

For legibility, this is all desired behavior IMO.
I can do something about the flickering, however, when the background color changes - I'll transition it from black to white and vice-versa to make it easier on the eyes. I've tested this locally and looks very nice.

The other thing, when Highlight All is clicked repeatedly, is another matter and will be fixed with the item above.
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Iteration: --- → 52.2 - Oct 17
Points: --- → 2
Flags: qe-verify+
Flags: firefox-backlog+
Comment on attachment 8797558 [details]
Bug 1306234 - stop flickering when the background color of the findbar dimmed highlighting mode changes and make sure the color changes if needed when all the ranges have been found as well.

https://reviewboard.mozilla.org/r/83242/#review82850

So what's happening here is that the dark background is used because the first match is on a dark background, but then when the highlight all is toggled the other matches were taken into account and the code had determined not to darken the page as much?

::: toolkit/modules/FinderHighlighter.jsm:77
(Diff revision 2)
>    ],
>    maskNode: [
>      ["background", "rgba(0,0,0,.35)"],
>      ["pointer-events", "none"],
>      ["position", "absolute"],
> +    ["transition", "background .5s ease-in"],

This transition is a bit long here. Can you lower this to .2s ? I'm also a bit worried that this will degrade performance. Have you tested this on large pages?
Attachment #8797558 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> This transition is a bit long here. Can you lower this to .2s ?

Done!

> I'm also a
> bit worried that this will degrade performance. Have you tested this on
> large pages?

Good point! I made a small adjustment so that the transition is only enabled when the rects are drawn as well (meaning that the initial, first time render will not trigger a transition).
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a4c3d4d2588c
stop flickering when the background color of the findbar dimmed highlighting mode changes and make sure the color changes if needed when all the ranges have been found as well. r=jaws
https://hg.mozilla.org/mozilla-central/rev/a4c3d4d2588c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
QA Contact: brindusa.tot
Verified fixed on the latest Nightly 52.0a1 (Build ID:20161013030204) on Ubuntu 16.04 and Windows 10 and Mac OS X 10.11
I can still see a short reverse for the dimmed background after "Highlight All" button is pressed.For this I filed bug 1310161.

Considering the above, setting the status verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.