Closed Bug 1655398 Opened 6 months ago Closed 6 months ago

`document.documentElement.clientTop` return an incorrect value

Categories

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

80 Branch
defect

Tracking

()

VERIFIED FIXED
81 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox78 --- unaffected
firefox79 --- unaffected
firefox80 --- verified
firefox81 --- verified

People

(Reporter: nayinain, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Attached file 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.

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

Has Regression Range: --- → yes
Has STR: --- → yes
Keywords: regression
Regressed by: 1654769

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

Assignee: nobody → emilio
Status: UNCONFIRMED → NEW
Ever confirmed: true

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.

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/31ebee098292
Don't move the client area rect for the root scroll frame. r=mats
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24769 for changes under testing/web-platform/tests

(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).

Upstream PR merged by moz-wptsync-bot
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1fa3c63f82d2
Don't move the client area rect for the root scroll frame. r=mats
Failed to create upstream wpt PR due to merge conflicts. This requires fixup from a wpt sync admin.

(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.

Flags: needinfo?(emilio) → needinfo?(james)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/24781 for changes under testing/web-platform/tests

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

Flags: needinfo?(james)
Upstream PR merged by moz-wptsync-bot
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch
Duplicate of this bug: 1655811
Duplicate of this bug: 1655855
Duplicate of this bug: 1655830
Duplicate of this bug: 1655950

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.

Flags: needinfo?(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
Flags: needinfo?(emilio)
Attachment #9166231 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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

Attachment #9166231 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1656710
QA Whiteboard: [qa-triaged]
Duplicate of this bug: 1656880
Duplicate of this bug: 1656800

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)

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.