|Summary:||Drop non-standard [IsIndex] WebKit IDL extended attribute|
|Product:||WebKit||Reporter:||Chris Dumez <cdumez>|
|Component:||Bindings||Assignee:||Chris Dumez <cdumez>|
|Severity:||Normal||CC:||ap, darin, ggaren, rniwa, sam, webkit-bug-importer|
Description Chris Dumez 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.
Comment 4 Darin Adler 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".
Comment 5 Darin Adler 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.
Comment 6 Chris Dumez 2015-09-14 18:48:51 PDT
Committed r189770: <http://trac.webkit.org/changeset/189770>
Comment 7 Chris Dumez 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.