Deprecate Element.offsetParent/offsetTop/offsetLeft/offsetWidth/offsetHeight

Deprecate Element.offsetParent/offsetTop/offsetLeft/offsetWidth/offsetHeight

Intent to Deprecate: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/Hi2ni3HdXTw

BUG=504578, 463116
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=198857

[philipj_slow, 2015-07-08 12:16:18]

philipj@opera.com changed reviewers:
+ philipj@opera.com

[philipj_slow, 2015-07-08 12:16:20]

https://codereview.chromium.org/1231533002/diff/20001/Source/core/dom/Element...
File Source/core/dom/Element.idl (left):

https://codereview.chromium.org/1231533002/diff/20001/Source/core/dom/Element...
Source/core/dom/Element.idl:99: // FIXME: offset* should only be on HTMLElement.
Add crbug.com/504578 to the end of this comment.

https://codereview.chromium.org/1231533002/diff/20001/Source/core/frame/UseCo...
File Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1231533002/diff/20001/Source/core/frame/UseCo...
Source/core/frame/UseCounter.cpp:951: return "'Element.offsetParent' is
deprecated and will be removed in M47, around Novemver 2015. The offset*
attributes are only standardized and widely supported for HTML elements";
There should be a period at the end of this message.

[tanay.c, 2015-07-10 09:07:20]

Modified as per review comments. PTAL.

https://codereview.chromium.org/1231533002/diff/20001/Source/core/dom/Element...
File Source/core/dom/Element.idl (left):

https://codereview.chromium.org/1231533002/diff/20001/Source/core/dom/Element...
Source/core/dom/Element.idl:99: // FIXME: offset* should only be on HTMLElement.
On 2015/07/08 12:16:20, philipj wrote:
> Add crbug.com/504578 to the end of this comment.

Done.

https://codereview.chromium.org/1231533002/diff/20001/Source/core/frame/UseCo...
File Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1231533002/diff/20001/Source/core/frame/UseCo...
Source/core/frame/UseCounter.cpp:951: return "'Element.offsetParent' is
deprecated and will be removed in M47, around Novemver 2015. The offset*
attributes are only standardized and widely supported for HTML elements";
On 2015/07/08 12:16:20, philipj wrote:
> There should be a period at the end of this message.

Done.

[philipj_slow, 2015-07-10 09:39:33]

Looks like there isn't a single test that hits the deprecation messages. Can you
add a test that does nothing but create a non-HTML/SVG element and accesses
these attributes to verify that the deprecation messages show up? I know it
seems silly, but it is possible to fail with use counters, and I have done so
more than once, i.e. twice :)

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
File Source/core/svg/SVGElement.idl (right):

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
Source/core/svg/SVGElement.idl:35: [Measure, PerWorldBindings] readonly
attribute Element? offsetParent;
Document this as

// TODO(tanay.c): SVGElement.offset* are non-standard. crbug.com/508865

[philipj_slow, 2015-07-10 11:16:30]

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
File Source/core/svg/SVGElement.idl (right):

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
Source/core/svg/SVGElement.idl:35: [Measure, PerWorldBindings] readonly
attribute Element? offsetParent;
On 2015/07/10 09:39:33, philipj wrote:
> Document this as
> 
> // TODO(tanay.c): SVGElement.offset* are non-standard. crbug.com/508865

Make that crbug.com/463116 and also use BUG=463116,504578 so that this change
will shop up there.

[philipj_slow, 2015-07-14 08:48:14]

On 2015/07/10 09:39:33, philipj wrote:
> Looks like there isn't a single test that hits the deprecation messages. Can
you
> add a test that does nothing but create a non-HTML/SVG element and accesses
> these attributes to verify that the deprecation messages show up? I know it
> seems silly, but it is possible to fail with use counters, and I have done so
> more than once, i.e. twice :)

To reply to the question from IRC, such a test might look like this, using
testharness.js:

test(function() {
  var element = document.createElementNS(null, 'x');
  element.offsetParent;
  element.offsetTop;
  element.offsetLeft;
  element.offsetWidth;
  element.offsetHeight;
});

[tanay.c, 2015-07-14 11:02:46]

Added the tests. Please have a look.

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
File Source/core/svg/SVGElement.idl (right):

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
Source/core/svg/SVGElement.idl:35: [Measure, PerWorldBindings] readonly
attribute Element? offsetParent;
On 2015/07/10 09:39:33, philipj wrote:
> Document this as
> 
> // TODO(tanay.c): SVGElement.offset* are non-standard. crbug.com/508865

Acknowledged.

https://codereview.chromium.org/1231533002/diff/100001/Source/core/svg/SVGEle...
Source/core/svg/SVGElement.idl:35: [Measure, PerWorldBindings] readonly
attribute Element? offsetParent;
On 2015/07/10 11:16:30, philipj wrote:
> On 2015/07/10 09:39:33, philipj wrote:
> > Document this as
> > 
> > // TODO(tanay.c): SVGElement.offset* are non-standard. crbug.com/508865
> 
> Make that crbug.com/463116 and also use BUG=463116,504578 so that this change
> will shop up there.

Done.

[philipj_slow, 2015-07-14 11:08:42]

The CQ bit was checked by philipj@opera.com

[philipj_slow, 2015-07-14 11:08:43]

lgtm

[commit-bot: I haz the power, 2015-07-14 11:08:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1231533002/110001

[commit-bot: I haz the power, 2015-07-14 12:30:37]

Committed patchset #7 (id:110001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198857