Closed Bug 1495169 Opened 1 year ago Closed 1 year ago

Page top/bottom part is truncated after change width of content area if you keep moving the mouse pointer over the content area

Categories

(Core :: Layout, defect, P3, major)

defect

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox-esr60 --- wontfix
firefox62 --- wontfix
firefox63 --- verified
firefox64 --- verified

People

(Reporter: alice0775, Assigned: emilio)

References

(Depends on 1 open bug)

Details

(Keywords: regression)

Attachments

(3 files)

Firefox62.0.2  : Can not reproduce with the following str
Firefox63.0b10 : Cat not reproduce with the following str
Nightly64.0a1  : Reproduce


Reproducible: almost always

Steps To Reproduce:

1. Browser width to be 1280px
2. Open Sidebar(ctrl+B)
3. Open https://www.ecma-international.org/ecma-262/8.0/index.html
4. Wait for page loading
5. Scroll to bottom
6. Wait to render
7. Close Sidebar(ctrl+B)
   --- Bug appears, now, page bottom part is truncated
8. Click on "G Copyright & Software License" at bottom of left side menu
   --- Page bottom part appears
9. Scroll to top
   --- Bug appears, now, page top part is truncated


Actual Results:
Page top and bottom part is truncated.
Unable to scroll to page top and bottom.


Expected Results:
It should not be truncated.
It can scroll to page top and bottom
Attached file about:support
I can see the issue on Linux. Setting P3 tentatively.
OS: Windows 10 → All
Priority: -- → P3
Hardware: x86_64 → All
On 63.0b10, reproduce the issue with the following STR

STR
1. Browser width to be 1280px
2. Open Sidebar(ctrl+B)
3. Open https://www.ecma-international.org/ecma-262/8.0/index.html
4. Wait for page loading
5. Scroll to bottom
6. Wait to render
7. Close Sidebar(ctrl+B)
8. Scroll to bottom
   --- OK, you can see page bottom part as expected
9. Open Sidebar(ctrl+B)
10. Scroll to bottom
   --- Bug appears, now, page bottom part is truncated

11. if not reproduce, repeat from step 7
It may be necessary to repeat STR of comment#4 3-5 times on 63.0b3
It seems that a bug occurs if you keep moving the mouse pointer over the content area.

Reproduced: 60.2.1esr, 62.0.2, 63.0b10, Nightly64.0a1


STR
1. Browser width to be 1280px
2. Open Sidebar(ctrl+B)
3. Open https://www.ecma-international.org/ecma-262/8.0/index.html
4. Wait for page loading
5. Scroll to bottom
6. Wait for render

7. Close Sidebar(ctrl+B)
8. Scroll to bottom
9. Open Sidebar(ctrl+B)
10. you keep moving the mouse pointer over the content area until redrawing is completed
11. Scroll to bottom
   --- Bug appears, now, page bottom part is truncated

11. if not reproduce, repeat from step 7



If you set layout.interruptible-reflow.enabled to false, the problem does not seem to happen.
e10s is not matter. I can reproduce the issue with and without e10s both.
Blocks: ireflow
Summary: Page top/bottom part is truncated after change width of content area → Page top/bottom part is truncated after change width of content area if you keep moving the mouse pointer over the content area
So as long as you keep moving the mouse we will interrupt reflows; that part is expected.  But you're saying that you stop moving the mouse and the rendering still does not update?
(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #9)
> So as long as you keep moving the mouse we will interrupt reflows; that part
> is expected.  But you're saying that you stop moving the mouse and the
> rendering still does not update?

Yes I can reproduce the issue on Nightky64.0a1 without mouse moving.
Daniel, do you have time to take a look or know someone on layout who does?
Flags: needinfo?(dholbert)
This looks very similar to other bug that was reported recently (can't find it r/n). My theory is that a measuring reflow for a flex item gets interrupted and we incorrectly cache the measurement there, then we incorrectly use it for other non-interruptible reflows.

Boris, is checking HasPendingInterrupt at the beginning of reflow Reflow() and in DidReflow() enough to tell whether any of our kids was interrupted?

If so, we should probably just clear the cached measuring reflow in that case to avoid using it in subsequent reflows.
Flags: needinfo?(bzbarsky)
> is checking HasPendingInterrupt at the beginning of reflow Reflow() and in DidReflow() enough to tell whether any of our kids was interrupted?

I think so, yes.
Flags: needinfo?(bzbarsky)
If we're waiting on an interrupt, then our child items haven't been totally
reflowed and our measures would be bogus.

This will probably regress performance in the cases bug 1209697 fixed, so we
should probably add an interrupt check somewhere in nsFlexContainerFrame to
avoid keeping reflowing flex containers indefinitely.

We could probably just bail out from our reflow if any kid reflow was
interrupted.
Alice can you check whether one of the builds from https://treeherder.mozilla.org/#/jobs?repo=try&revision=66f4b14d9a89f6068e492ec490dfd7e500a635ad fixes the issue? Thanks!
Flags: needinfo?(alice0775)
> We could probably just bail out from our reflow if any kid reflow was interrupted.

If we can do that, it would be ideal.
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
> Alice can you check whether one of the builds from
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=66f4b14d9a89f6068e492ec490dfd7e500a635ad fixes the
> issue? Thanks!

Actually, https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f4068771c55cd909ce5f2806bca73cfa79ffc48, since the build above is probably slightly broken (forgot to call the base class method, sigh).
(In reply to Emilio Cobos Álvarez (:emilio) from comment #15)
> Alice can you check whether one of the builds from
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=66f4b14d9a89f6068e492ec490dfd7e500a635ad fixes the
> issue? Thanks!


The try build fixes the issue!
Flags: needinfo?(alice0775)
[Canceling ni & assigning to Emilio, since it looks like he's got this (thanks Emilio!)]
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Flags: needinfo?(dholbert)
(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)
> Actually,
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=5f4068771c55cd909ce5f2806bca73cfa79ffc48, since the
> build above is probably slightly broken (forgot to call the base class
> method, sigh).

The try build(x64 pgo, x86 opt) also fixes this issue.
Depends on: 1495532
Attachment #9013310 - Attachment description: Bug 1495169 - WIP: Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted. → Bug 1495169 - Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/6daf79d1bace
Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted. r=dholbert
https://hg.mozilla.org/mozilla-central/rev/6daf79d1bace
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Is this something we should consider for Beta backport with the RCs being next week?
Flags: needinfo?(emilio)
Comment on attachment 9013310 [details]
Bug 1495169 - Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted.

It's safe I'd say, it's only clearing cached stuff.

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1209697

User impact if declined: Some big pages may be wrongly sized if interacted with at the wrong time.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: Yes

If yes, steps to reproduce: See comment 0, or bug 1362758.

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Only clearing cached stuff when we get interrupted, so should have ~0 risk.

String changes made/needed: none
Flags: needinfo?(emilio)
Attachment #9013310 - Flags: approval-mozilla-beta?
Comment on attachment 9013310 [details]
Bug 1495169 - Remove cached measuring reflows if we aren't measuring the right thing because we got interrupted.

Minimal patch fixing a 63 regression, evaluated to very low risk, uplift approved for our last 63 beta, thanks.
Attachment #9013310 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
I’ve reproduce this issue on Fx 63.0b10 (20180927143327) with Windows 10 x64, by following the steps provided in comment 6.
On Fx 63.0b14 (20181011200118) and Fx 64.0a1 (2018-10-11), using Windows 10 x64, Ubuntu 16.04 x86 and macOS 10.13, the issue is no longer reproducible.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.