Bug 149122 - Drop non-standard [IsIndex] WebKit IDL extended attribute
Summary: Drop non-standard [IsIndex] WebKit IDL extended attribute
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Bindings (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2015-09-14 11:16 PDT by Chris Dumez
Modified: 2015-09-14 19:01 PDT (History)
6 users (show)

See Also:


Attachments
Patch (38.92 KB, patch)
2015-09-14 11:42 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (28.22 KB, patch)
2015-09-14 12:49 PDT, Chris Dumez
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.