Closed Bug 1566783 Opened 4 months ago Closed 4 months ago

anchor jumps to incorrect position if the link is opened in foreground tab

Categories

(Core :: Layout: Scrolling and Overflow, defect, P3)

70 Branch
Desktop
Windows 10
defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- verified
firefox68 --- wontfix
firefox69 --- verified
firefox70 --- verified

People

(Reporter: alice0775, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: nightly-community, regression, reproducible)

Attachments

(3 files)

  • setting layout.css.scroll-anchoring.enabled=false fixes the issue.
  • Bug 1546027 does not fix this issue.

Reproducible: always

Steps to reproduce:

  1. Load https://bug1546027.bmoattachments.org/attachment.cgi?id=9059735#anc2
  2. Ctrl+Shift+Click a link( e.g "Beta") to open in foreground new tab
    --- observe scroll position
  3. Ctrl+Click a link( e.g "Beta") to open in background new tab
    --- observe scroll position

Actual Results:
anchor jumps to incorrect position if the link is opened in foreground tab.
i.e, the image is displayed partially.

Expected results:
anchor jumps should be correct position even if the link is opened in foreground tab.
i.e, the whole image should be displayed.

Thanks for filing Alice :)

Assignee: nobody → emilio
Priority: -- → P3
Depends on: 1524281

Seems we can leave this node alive for too long if the user scrolls between
domcontentloaded (where GoToAnchor is called) and onload (where ScrollToAnchor()
is called).

Though it seems we can leave it for too long if we don't end up calling
ScrollToAnchor(), the documentation of the method claims that it's cleared
unconditionally.

Depends on D38397

:emilio, since this bug is a regression, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Regressed by: 1524281
Blocks: 1566621
Attachment #9078846 - Attachment description: Bug 1566783 - Don't prevent PresShell::ScrolToAnchor() from working due to scroll anchoring adjustments that happen without the user scrolling. r=dholbert → Bug 1566783 - Don't prevent PresShell::ScrollToAnchor() from working due to scroll anchoring adjustments that happen without the user scrolling. r=dholbert
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/413557ff3269
Don't prevent PresShell::ScrollToAnchor() from working due to scroll anchoring adjustments that happen without the user scrolling. r=dholbert
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af346beee1df
Make sure to clear mLastAnchorScrolledTo after calling PresShell::ScrollToAnchor(). r=dholbert
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Flags: in-testsuite+

Comment on attachment 9078846 [details]
Bug 1566783 - Don't prevent PresShell::ScrollToAnchor() from working due to scroll anchoring adjustments that happen without the user scrolling. r=dholbert

Beta/Release Uplift Approval Request

  • User impact if declined: Scroll to anchor fails when something changes above the anchor node before onload.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: comment 0 / comment 1.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): One liner that fixes a scroll anchoring regression that has a couple dupes.
  • String changes made/needed: none

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Scroll anchoring regression.
  • User impact if declined: See above
  • Fix Landed on Version: 70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): see above
  • String or UUID changes made by this patch: none
Attachment #9078846 - Flags: approval-mozilla-esr68?
Attachment #9078846 - Flags: approval-mozilla-beta?
Attachment #9078847 - Flags: approval-mozilla-esr68?
Attachment #9078847 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9078846 [details]
Bug 1566783 - Don't prevent PresShell::ScrollToAnchor() from working due to scroll anchoring adjustments that happen without the user scrolling. r=dholbert

Fixes a scroll anchoring regression which has picked up some duplicate reports. Includes an automated test. Approved for 69.0b9 and 68.1esr.

Attachment #9078846 - Flags: approval-mozilla-esr68?
Attachment #9078846 - Flags: approval-mozilla-esr68+
Attachment #9078846 - Flags: approval-mozilla-beta?
Attachment #9078846 - Flags: approval-mozilla-beta+
Attachment #9078847 - Flags: approval-mozilla-esr68?
Attachment #9078847 - Flags: approval-mozilla-esr68+
Attachment #9078847 - Flags: approval-mozilla-beta?
Attachment #9078847 - Flags: approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I’ve reproduced this issue with Fx 70.0a1 (2019-07-17) on Windows 10 x64.
The issue is verified fixed with Fx 70.0a1 (2019-07-29), Fx 69.0b9 (20190730004747) and Fx 68.1.0esr (20190729203625) on Windows 10 x64, macOs 10.14 and Ubuntu 18.04 x64.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Duplicate of this bug: 1566621
You need to log in before you can comment on or make changes to this bug.