1633192 - "scroll-margin" has no effect when "display: table"

Status: RESOLVED FIXED

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

Description

[Matthias Geier]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

A link target element has "scroll-margin-top: 4em" (or whatever length) and "display: table".

Here's an example: https://jsbin.com/cocopasada/edit?html,css,output
"link1" works as expected, "link2" shows the erroneous behavior.

Actual results:

The target element is scrolled to the top of the page, but "scroll-margin-top" is not taken into account.

Expected results:

When "display" is anything else, "scroll-margin-top" is taken into account and the element is scrolled to the given amount below the top of the page.

I would expect the same behavior for "display: table".

Comment 1

[Gyula Palko[:gpalko]]

Hi, I'm setting the component in order to involve the development team.
Thanks for reporting!

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Probably the table wrapper should inherit scroll-margin. This is probably a matter of either:

Tweaking this line to check nsLayoutUtils::GetStyleFrame or, alternatively adding scroll-margin: inherit to this CSS rule.

Then verify that it fixes the bug, and write a test.

Comment 3

[Siv2r]

Hi,
I am new to Opensource culture. I am interested in solving this bug.
can this bug be assigned to me?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Sure, please go ahead!

Comment 5

[Firefox Bug Husbandry Bot]

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)

Comment 6

[Jan]

Hey, I am currently trying to fix this. After adding scroll-margin: inherit and rebuilding firefox the example works as expected. How to I continue from now? It seems the next step would be to write a test. Should a follow this guide to do so?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Tests for web platform features (like this) should go into WPT.

We have some tests for scroll-margin here, you can probably copy that test making #target a display: table, after ensuring the test fails before your patch and passes after it.

Comment 8

[Jan]

Attachment: Bug 1633192 - Added 'scroll-margin: inherit;' to ua.css to make scroll-margin work on elements with 'display: table'. r?emilio

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c7b1f73f0a11
Added 'scroll-margin: inherit;' to ua.css to make scroll-margin work on elements with 'display: table'. r=emilio

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/23752 for changes under testing/web-platform/tests

Comment 11

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c7b1f73f0a11

Comment 13

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Thanks for fixing this Jan!

Comment 15

[Jan]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #14)

Thanks for fixing this Jan!

Thank you for guiding me through the process!