1305725 - Emphasized highlight is wrongly positioned after multiple browser resize

Status: RESOLVED WORKSFORME

(Toolkit :: Find Toolbar, defect)

Description

[roxleitan]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0 
Build ID: 20160927030200

[Affected versions]:
Nightly 52.0a1

[Affected platforms]:
Ubuntu 16.04 x64, Windows 10 x64

[Steps to reproduce]:
1.Launch Nightly 52.0a1 with a new profile
2.Open https://www.washingtonpost.com/
3.Open Find Bar (Ctrl+F)
4.Search for "Of" 
5.Resize browser multiple times by dragging browser margins using the mouse 

[Expected result]:
Emphasized highlight should follow browser resize

[Actual result]:
Emphasized highlight is wrongly positioned

Comment 1

[Mike de Boer [:mikedeboer]]

Attachment: Bug 1305725 - remove iffy content change detection and paint selectively after each paint. Also implement a slow tailing repaint sweep to anticipate slow reflows.

Review commit: https://reviewboard.mozilla.org/r/97734/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/97734/

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8817493 [details]
Bug 1305725 - remove iffy content change detection and paint selectively after each paint. Also implement a slow tailing repaint sweep to anticipate slow reflows.

https://reviewboard.mozilla.org/r/97734/#review98732

r=me with the below issues addressed.

::: toolkit/modules/FinderHighlighter.jsm:1139
(Diff revision 1)
>  
>      this._updateRangeOutline(dict);
>  
>      let allRects = [];
>      if (paintContent || dict.modalHighlightAllMask) {
> +      // No need to update dynamic ranges separately when we already about to

nit, s/when we already/when we are already/

::: toolkit/modules/FinderHighlighter.jsm:1249
(Diff revision 1)
>          dict.unconditionalRepaintRequested = false;
>          this._repaintHighlightAllMask(window);
> +
> +        // Make sure to do a sweep repaint of the highlights after delayed, JS
> +        // and timer based content reflows that take more time to complete than
> +        // 'MozAfterPaint' to fire. See bug 1305725 for more info.

There isn't really more info in bug 1305725 right now. Can you explain more there? Otherwise this comment might as well explain it.

::: toolkit/modules/FinderHighlighter.jsm:1252
(Diff revision 1)
> +        // Make sure to do a sweep repaint of the highlights after delayed, JS
> +        // and timer based content reflows that take more time to complete than
> +        // 'MozAfterPaint' to fire. See bug 1305725 for more info.
> +        dict.modalRepaintSweepScheduler = window.setTimeout(
> +          this._scheduleRepaintOfMask.bind(this, window, { updateAllRanges: true }),
> +          kModalHighlightRepaintLoFreqMs * 2);

Why are we doubling kModalHighlightRepaintLoFreqMs? 

I'm worried that this effectively gets us away from the constant and down the road we could have multiple places that use different multiples of the constant without clear explanation as to why.

Comment 3

[Julien Cristau [:jcristau]]

Too late for firefox 52, mass-wontfix.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 5

[Giorgia Nichita, Release Desktop QA]

I am closing this as WFM as it is no longer reproducible with our latest nightly build 132.0a1 (2024-09-23) using Win 10, Ubuntu 22 and macOS 11.