New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 719246 link

Starred by 5 users
Status: Fixed
Owner:
fs...@chromium.org
Closed: Jul 2017
Cc:
█ gog...@gmail.com
jinho.b...@samsung.com
fs...@chromium.org
foolip@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug
Team-Blink-Paint



Sign in to add a comment

Make ClientRect an alias of DOMRect.

Reported by gog...@gmail.com, May 7 2017 
DOMRect was changed sufficiently that ClientRect can be made a simple alias of it.

Rough steps:

1. Sync DOMRect with https://drafts.fxtf.org/geometry/#DOMRect (it's not shipped, so entirely safe)
2. Remove the ClientRect code in favor of an alias for DOMRect
3. Use Sequence instead of ClientRectList code
4. Watch for regressions, revert, and tweak the spec until it works

Comment 1 by fs...@chromium.org, May 8 2017

Cc: fs...@chromium.org
What's the status on this?

Comment 2 by jinho.b...@samsung.com, May 9 2017 

I'm not sure but he is working on these CLs (under my review):
https://codereview.chromium.org/2870513002/
https://codereview.chromium.org/2869803003/

Comment 3 by gog...@gmail.com, May 9 2017 

I am working on this.
Plz check below link.

https://codereview.chromium.org/2869803003/
https://codereview.chromium.org/2870513002/

Comment 4 by sim...@opera.com, May 11 2017 

The spec does not have ClientRect as an alias, but it could be added... it has other aliases already, although apart from WebKitCSSMatrix it's not clear to me if they're really needed for web compat.

Comment 5 by tkent@chromium.org, May 11 2017

Components: -Blink>DOM Blink>Geometry

Comment 6 by fs...@chromium.org, May 15 2017

Blocking: -388780
Since this is not spec'ed out properly yet, I'll drop the blocking, but keep the issue.

Comment 7 by foolip@chromium.org, May 16 2017 

Hmm, we should probably just try to rename ClientRect to DOMRect and ship without an alias. If that doesn't work out, that's a strong argument for adding an alias to the spec.

Comment 8 by sim...@opera.com, May 16 2017 

I filed https://github.com/w3c/fxtf-drafts/issues/163 to track this on the spec side.

I agree that minimizing the number of aliases is desirable; at the same time it seems pretty cheap to have them, so... meh :-)

Comment 9 by foolip@chromium.org, May 17 2017 

The main reason to try without an alias is that Gecko doesn't have it. If there is any code out there that amounts to "var isWebKit = !!window.ClientRect", then Gecko would take a risk by adding it.

I suppose that httparchive research would be in order, but it's quite time consuming.

Comment 10 by sim...@opera.com, May 18 2017 

Analysis at https://github.com/w3c/fxtf-drafts/issues/163#issuecomment-302332055

It seems it should be possible to not have an alias for ClientRect.

Comment 11 by schenney@chromium.org, Jun 26 2017

Cc: gog...@gmail.com
Owner: ----
Status: Available (was: Started)
Owner inactive last 30 days. Marking Available. Re-assign if that is incorrect.

Comment 12 by fs...@chromium.org, Jun 28 2017

Owner: fs...@chromium.org
I'm a bit confused about this change still.
Does this mean getting rid of core/dom/ClientRect? Then what happens to getClientRects() and getBoundingClientRects() ?

Comment 13 by foolip@chromium.org, Jun 30 2017 

The change is to remove the ClientRect type, and make getBoundingClientRect() return a DOMRect instead.

Comment 14 by foolip@chromium.org, Jun 30 2017 

(In the same milestone that ships DOMRect.)

Comment 15 by fs...@chromium.org, Jun 30 2017 

and replace ClientRectList with a Vector<DOMRect> ?

Comment 16 by foolip@chromium.org, Jun 30 2017 

No, looks like DOMRectList is making a comeback: https://github.com/w3c/fxtf-drafts/pull/186

Comment 17 by fs...@chromium.org, Jun 30 2017 

Oh. So it means also change ClientRectList into DOMRectList, right?

Comment 18 by foolip@chromium.org, Jun 30 2017 

Yep.

Comment 19 by fs...@chromium.org, Jul 10 2017

Status: Started (was: Available)
in CQ.
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 10 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f01e385c3f752c2bfd1163db74ecbf88e5390d25

commit f01e385c3f752c2bfd1163db74ecbf88e5390d25
Author: Fernando Serboncini <fserb@chromium.org>
Date: Mon Jul 10 23:14:46 2017

Replaces ClientRect/ClientRectList with DOMRect/DOMRectList

Related spec: https://drafts.fxtf.org/geometry/#DOMRect
and: https://drafts.fxft.org/geometry/#DOMRectList

Bug:  719246 
Change-Id: I0322c12ab5c327b869e925a8c06fbfae6b659ad3
Reviewed-on: https://chromium-review.googlesource.com/559978
Commit-Queue: Fernando Serboncini <fserb@chromium.org>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485438}
[delete] https://crrev.com/f23b896efce3a6ea139d29efd791bc09974def9f/third_party/WebKit/LayoutTests/external/wpt/css/geometry-1/historical-expected.txt
[delete] https://crrev.com/f23b896efce3a6ea139d29efd791bc09974def9f/third_party/WebKit/LayoutTests/external/wpt/cssom-view/cssom-getBoundingClientRect-002-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/fast/dom/Element/client-rect-list-argument-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/fast/dom/Element/client-rect-list-argument.html
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/platform/linux/svg/zoom/page/zoom-zoom-coords-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/platform/win/svg/zoom/page/zoom-zoom-coords-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/core_idl_files.gni
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/BUILD.gn
[delete] https://crrev.com/f23b896efce3a6ea139d29efd791bc09974def9f/third_party/WebKit/Source/core/dom/ClientRect.cpp
[delete] https://crrev.com/f23b896efce3a6ea139d29efd791bc09974def9f/third_party/WebKit/Source/core/dom/ClientRect.h
[delete] https://crrev.com/f23b896efce3a6ea139d29efd791bc09974def9f/third_party/WebKit/Source/core/dom/ClientRect.idl
[delete] https://crrev.com/f23b896efce3a6ea139d29efd791bc09974def9f/third_party/WebKit/Source/core/dom/ClientRectList.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Element.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Element.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/ElementTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/IntersectionObserverEntry.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/IntersectionObserverEntry.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/IntersectionObserverEntry.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Range.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Range.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/Range.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/ResizeObserverEntry.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/ResizeObserverEntry.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/dom/ResizeObserverEntry.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/frame/BrowserControlsTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/frame/DocumentLoadingRenderingTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/BUILD.gn
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRect.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRect.h
[rename] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRectList.cpp
[rename] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRectList.h
[add] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRectList.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRectReadOnly.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/geometry/DOMRectReadOnly.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/html/media/MediaRemotingElements.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/html/track/vtt/VTTRegion.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/input/TouchActionTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/inspector/InspectorHighlight.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/layout/ScrollAnchorTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/page/Page.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/page/Page.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/page/scrolling/RootScrollerTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/Internals.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/Internals.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/LayerRect.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/LayerRect.idl
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/LayerRectList.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/core/testing/LayerRectList.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/modules/media_controls/MediaControlsImplTest.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/modules/media_controls/elements/MediaControlCastButtonElement.cpp
[modify] https://crrev.com/f01e385c3f752c2bfd1163db74ecbf88e5390d25/third_party/WebKit/Source/web/tests/ScrollMetricsTest.cpp

Comment 21 by tkent@chromium.org, Jul 11 2017 

 Issue 714656  has been merged into this issue.

Comment 22 by fs...@chromium.org, Jul 11 2017

Status: Fixed (was: Started)
Donerino. :)
►

Sign in to add a comment