RESOLVED FIXED 149122
Drop non-standard [IsIndex] WebKit IDL extended attribute
https://bugs.webkit.org/show_bug.cgi?id=149122
Summary Drop non-standard [IsIndex] WebKit IDL extended attribute
Chris Dumez
Reported 2015-09-14 11:16:40 PDT
Drop non-standard [IsIndex] WebKit IDL extended attribute. This attributes causes us to throw an IndexSizeError if the input value is negative. Web IDL supports no such thing. Instead Web IDL supports: 1. Default behavior: the input value wraps around if it does not fit. 2. [EnforceRange]: A TypeError is thrown if the input value does not fit (https://heycam.github.io/webidl/#EnforceRange) 3. [Clamp]: The input value will be clamped if it does not fit (https://heycam.github.io/webidl/#Clamp) Our bindings generator supports all three. We don't need the non-standard [IsIndex]. We currently use [IsIndex] in places where we're supposed to wrap around as per Web IDL. Therefore, we throw for negative values but other browsers don't. For e.g., CharacterData.substringData(offset, -1) is supposed to return the substring from offset to the end of the string. It does so in Firefox and Chrome. However, WebKit throws an Exception.
Attachments
Patch (38.92 KB, patch)
2015-09-14 11:42 PDT, Chris Dumez
no flags
Patch (28.22 KB, patch)
2015-09-14 12:49 PDT, Chris Dumez
darin: review+
Chris Dumez
Comment 1 2015-09-14 11:17:33 PDT
Chris Dumez
Comment 2 2015-09-14 11:42:31 PDT
Chris Dumez
Comment 3 2015-09-14 12:49:34 PDT
Darin Adler
Comment 4 2015-09-14 18:46:35 PDT
Comment on attachment 261121 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=261121&action=review > Source/WebCore/ChangeLog:27 > + for negative values and we know the value around instead, as other typo here: "we know the value around" but we mean "we now wrap the value around".
Darin Adler
Comment 5 2015-09-14 18:48:02 PDT
This bug is really titled wrong. The real issue here is the change in behavior for these negative indexes. We are changing the behavior to match changes in the specifications and other browsers. As you can see, the old behavior was required by older DOM specifications. Changing the behavior removes the need for [IsIndex]. Actually dropping [IsIndex] is a cleanup step and less interesting, so shouldn’t be the bug title.
Chris Dumez
Comment 6 2015-09-14 18:48:51 PDT
Chris Dumez
Comment 7 2015-09-14 19:00:57 PDT
(In reply to comment #5) > This bug is really titled wrong. The real issue here is the change in > behavior for these negative indexes. We are changing the behavior to match > changes in the specifications and other browsers. As you can see, the old > behavior was required by older DOM specifications. > > Changing the behavior removes the need for [IsIndex]. Actually dropping > [IsIndex] is a cleanup step and less interesting, so shouldn’t be the bug > title. Sorry, I saw this comment after committing. You're right. I actually did not initially realize this used to be expected behavior. I have always known Web IDL to wrap around.
Note You need to log in before you can comment on or make changes to this bug.