Parsing of IntersectionObservers' "rootMargin" member is inconsistent
Reported by
que....@gmail.com,
Jun 10 2016
|
||||||
Issue description
UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2763.0 Safari/537.36
Steps to reproduce the problem:
1. Open console.
2. Execute following code:
(function () {
let obsrever = new IntersectionObserver(() => {}, {
rootMargin: '10px 80px 0px 60px 5px 10px'
});
return obsrever.rootMargin;
})();
What is the expected behavior?
An exception is thrown, because due to the spec (http://rawgit.com/WICG/IntersectionObserver/master/index.html#parse-a-root-margin) number of tokens
can't be greater than 4.
What went wrong?
Resulting value of 'rootMargin' is '10px 80px 0px 60px'.
Anything that goes after the fourth token is ignored, i.e. you can even set '10px 80px 0px 60px FOO BAR' which will be considered as a valid value.
Did this work before? No
Chrome version: 53.0.2763.0 Channel: dev
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 22.0 r0
Also, I don't know if that's a thing but you can provide an array like ['1px 1px'] and it will be casted to string.
,
Jun 10 2016
Ehm, there is probably a typo in ["1px","2px"] because toString conversion will give you a string with comma separated values: "1px,2px". Besides the value of margin will remain empty.
,
Jun 14 2016
Unable to repro this issue on Windows 7 for Google Chrome Dev Version - 53.0.2763.0 Screen-recording is attached. @que.etc: Could you please perform the steps mentioned beneath and let us know your observations. 1. Re-test the same on a clean profile [chrome://settings -> Add Person] Thank you.
,
Jun 14 2016
Maybe I've got you wrong, but I see just the describe issue: value is "10px 80px 0px 60px" while the expected behavior is a thrown exception. Stpe 3 in http://rawgit.com/WICG/IntersectionObserver/master/index.html#parse-a-root-margin If the length of tokens is 0 or greater than 4, return failure. I mean that it should throw an exception instead of ignoring the rest of tokens, isn't it?
,
Jun 14 2016
@rnimmagadda: Yeah, something like that. Or maybe a range error with information about the maximum number of tokens.
,
Jun 14 2016
Thank you for providing more feedback. Adding requester "rnimmagadda@chromium.org" for another review and adding "Needs-Review" label for tracking. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 15 2016
====================================== Good Build: 51.0.2700.0 Base Position: 385057 Bad Build: 51.0.2702.0 Base Position: 385602 ====================================== Able to repro this issue on Windows 7, MAC (10.11.5) & Ubuntu Trusty (14.04) for the Google Chrome Stable Version - 51.0.2704.84 This is a regression issue broken in M51, below mentioned is the bisect info: CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/7b7b6a937cec0bd9b8e30091493001dc3db2a0a6..7af31b5cfd45ea5e9646b835645c0e493a0c879c Suspecting Commit: 87fe5caf2ab2113388c6928c0f769525ba366d7e Review URL: https://codereview.chromium.org/1861753002 @szager: Could you please look into the issue, and if it has nothing to do with your changes and if possible please do assign it to the concerned owner. Thank you.
,
Jun 15 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f7fb6ab2ddb97b65193d9a12008124096d85c6b commit 1f7fb6ab2ddb97b65193d9a12008124096d85c6b Author: szager <szager@chromium.org> Date: Thu Jun 23 08:55:51 2016 IntersectionObserver constructor: fail on invalid rootMargin syntax. BUG= 618976 R=ojan@chromium.org,eae@chromium.org Review-Url: https://codereview.chromium.org/2088963002 Cr-Commit-Position: refs/heads/master@{#401562} [modify] https://crrev.com/1f7fb6ab2ddb97b65193d9a12008124096d85c6b/third_party/WebKit/LayoutTests/intersection-observer/observer-exceptions-expected.txt [modify] https://crrev.com/1f7fb6ab2ddb97b65193d9a12008124096d85c6b/third_party/WebKit/LayoutTests/intersection-observer/observer-exceptions.html [modify] https://crrev.com/1f7fb6ab2ddb97b65193d9a12008124096d85c6b/third_party/WebKit/LayoutTests/intersection-observer/root-margin-expected.txt [modify] https://crrev.com/1f7fb6ab2ddb97b65193d9a12008124096d85c6b/third_party/WebKit/LayoutTests/intersection-observer/root-margin.html [modify] https://crrev.com/1f7fb6ab2ddb97b65193d9a12008124096d85c6b/third_party/WebKit/Source/core/dom/IntersectionObserver.cpp
,
Jun 24 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by l446240525@gmail.com
, Jun 10 2016