Bug 149122

Summary: Drop non-standard [IsIndex] WebKit IDL extended attribute
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: BindingsAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, darin, ggaren, rniwa, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch darin: review+

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 1 Chris Dumez 2015-09-14 11:17:33 PDT
rdar://problem/22547139
Comment 2 Chris Dumez 2015-09-14 11:42:31 PDT
Created attachment 261117 [details]
Patch
Comment 3 Chris Dumez 2015-09-14 12:49:34 PDT
Created attachment 261121 [details]
Patch
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.