1655398 - `document.documentElement.clientTop` return an incorrect value

Status: VERIFIED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[nayinain]

Attachment: testcase.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:80.0) Gecko/20100101 Firefox/80.0

Steps to reproduce:

1. Open the testcase.html.

Actual results:

document.documentElement.clientTop return 100.

Expected results:

document.documentElement.clientTop should return 0.

Comment 1

[nayinain]

Regression window:

2020-07-27T13:29:28.313000: INFO : platform_changeset: 0ce4c53c6abb4e5325af3cd0505ec943d99e74a9
2020-07-27T13:29:28.313000: INFO : platform_repository: https://hg.mozilla.org/integration/autoland
2020-07-27T13:29:28.313000: INFO : platform_version: 80.0a1
2020-07-27T13:29:53.874000: INFO : Narrowed integration regression window from [2d3d30eb, 7916b0f7] (3 builds) to [2d3d30eb, 0ce4c53c] (2 builds) (~1 steps left)
2020-07-27T13:29:53.884000: DEBUG : Starting merge handling...
2020-07-27T13:29:53.884000: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=0ce4c53c6abb4e5325af3cd0505ec943d99e74a9&full=1
2020-07-27T13:29:53.884000: DEBUG : redo: attempt 1/3
2020-07-27T13:29:53.884000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/integration/autoland/json-pushes?changeset=0ce4c53c6abb4e5325af3cd0505ec943d99e74a9&full=1',), kwargs: {}, attempt #1
2020-07-27T13:29:53.886000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2020-07-27T13:29:55.926000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /integration/autoland/json-pushes?changeset=0ce4c53c6abb4e5325af3cd0505ec943d99e74a9&full=1 HTTP/1.1" 200 None
2020-07-27T13:29:55.964000: DEBUG : Found commit message:
Bug 1654769 - Make clientRect offsets relative to the primary frame of the element. r=mats

We're returning offsets relative to the scroll target frame, which for
inputs and such is not the primary frame, and thus they never have
border or padding or what not.

Offsets should be relative to the primary frame however, so move the
rect appropriately.

Differential Revision: https://phabricator.services.mozilla.com/D84919

2020-07-27T13:29:55.964000: DEBUG : Did not find a branch, checking all integration branches
2020-07-27T13:29:55.965000: INFO : The bisection is done.
2020-07-27T13:29:55.965000: INFO : Stopped

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Ugh, thanks for filing. You would thing these APIs that are implemented in all browsers since ages would have pretty good test coverage...

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1655398 - Don't move the client area rect for the root scroll frame. r=mats

The spec really asks us to put the rect at GetUsedBorder().{top,left} in
this case, but I don't think that really makes sense, see
https://github.com/w3c/csswg-drafts/issues/5363.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/31ebee098292
Don't move the client area rect for the root scroll frame. r=mats

Comment 5

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

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

Comment 6

[j.j.]

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

Ugh, thanks for filing. You would thing these APIs that are implemented in all browsers since ages would have pretty good test coverage...

The opposite is true:
It wasn't common practise to add tests for Gecko patches and implementations before ~ 2007 (and WPTs are much newer).

Comment 7

[Narcis Beleuzu [:NarcisB]]

Backed out for wpt failure on client-props-root.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/88230163c83760deeead9fbd33010d4d335cf676
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=311219005&repo=autoland&lineNumber=8442

Comment 8

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

Upstream PR merged by moz-wptsync-bot

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1fa3c63f82d2
Don't move the client area rect for the root scroll frame. r=mats

Comment 10

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

Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

(In reply to Web Platform Test Sync Bot (Matrix: #interop:mozilla.org) from comment #10)

Failed to create upstream wpt PR due to merge conflicts. This requires fixup
from a wpt sync admin.

James, it seems like https://github.com/web-platform-tests/wpt/pull/24769 was merged before the patch reached central? Or maybe the backout was merged at a different time than the patch? I did a minor tweak to the test.

Comment 12

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

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

Comment 13

[James Graham [:jgraham]]

Interesting; I wonder why it thought that it could merge this one. Anyway I poked it so the changes should be in that PR.

Comment 14

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

Upstream PR merged by moz-wptsync-bot

Comment 15

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1fa3c63f82d2

Comment 20

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

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9166231 [details]
Bug 1655398 - Don't move the client area rect for the root scroll frame. r=mats

Beta/Release Uplift Approval Request

• User impact if declined: various compat issues, see comment 0 and dupes.
• 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, any of the dupes.
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Pretty low risk patch that restores the previous behavior. Seems like the regressing patch snuck on beta.
• String changes made/needed: none

Comment 22

[Julien Cristau [:jcristau]]

Comment on attachment 9166231 [details]
Bug 1655398 - Don't move the client area rect for the root scroll frame. r=mats

approved for 80.0b3

Comment 23

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b317e3a71846

Comment 27

[Peter Magyari (Desktop QA)]

Reproduced the issue on Firefox 81.0a1 (20200727203201) and Firefox 80.0b2 (20200730152159)
Verified fixed on Firefox 81.0a1 (20200803213927) and Firefox 80.0b3 (20200803045446)