Bug 102515

Summary: Remove RangeException
Product: WebKit Reporter: Erik Arvidsson <arv>
Component: DOMAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, andersca, darin, gyuyoung.kim, haraken, japhet, ojan, rakuco, rniwa, sam, syoichi, webkit.review.bot
Priority: P2 Keywords: WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 102943    
Bug Blocks: 102505    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Erik Arvidsson 2012-11-16 08:05:13 PST
RangeException.cpp needs to be updated.

These errors should just be DOMExceptions.

http://dom.spec.whatwg.org/#dom-range-surroundcontents
Comment 1 Erik Arvidsson 2012-11-19 11:56:59 PST
This should be removed and we should throw DOMEception instead
Comment 2 Erik Arvidsson 2012-11-19 13:50:53 PST
Created attachment 175037 [details]
Patch
Comment 3 Erik Arvidsson 2012-11-19 13:52:21 PST
I'm not sure about the changes to ObjC and the Apple Windows port.
Comment 4 Build Bot 2012-11-19 16:42:58 PST
Comment on attachment 175037 [details]
Patch

Attachment 175037 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14909380

New failing tests:
http/tests/security/cross-frame-access-put.html
http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html
http/tests/misc/acid3.html
Comment 5 Sam Weinig 2012-11-19 17:00:23 PST
DOMRangeException is API on Mac, so you cannot remove it.  It would probably be acceptable never to throw it, but the header and class must remain.
Comment 6 Erik Arvidsson 2012-11-20 11:30:50 PST
Created attachment 175253 [details]
Patch
Comment 7 Adam Barth 2012-11-20 12:50:03 PST
Comment on attachment 175253 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175253&action=review

> Source/WebCore/dom/RangeException.idl:21
> +// This interface has been removed from the specs and is never exposed to the
> +// web. We keep this for the ObjC bindings.

Should we add a #if for LANGUAGE_OBJECTIVEC to have the compiler enforce this statement?

> LayoutTests/ChangeLog:34
> +        * http/tests/misc/acid3.html:

THis would be a good place to explain the changes you're making to acid3.

> LayoutTests/ChangeLog:42
> +        * http/tests/w3c/resources/testharness.js:

Has this change been made to testharness.js upstream in the W3C?

> LayoutTests/http/tests/misc/acid3.html:601
> +// COMMENTED OUT FOR 2011 UPDATE - we may want to merge RangeException and DOMException

Has this change been made to acid3 upstream?
Comment 8 Erik Arvidsson 2012-11-20 14:53:45 PST
(In reply to comment #7)
> (From update of attachment 175253 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=175253&action=review
> 
> > Source/WebCore/dom/RangeException.idl:21
> > +// This interface has been removed from the specs and is never exposed to the
> > +// web. We keep this for the ObjC bindings.
> 
> Should we add a #if for LANGUAGE_OBJECTIVEC to have the compiler enforce this statement?

Good idea.

> > LayoutTests/ChangeLog:34
> > +        * http/tests/misc/acid3.html:
> 
> THis would be a good place to explain the changes you're making to acid3.

Heh, I completely forgot. I was going to ;-) See below for answer.

> 
> > LayoutTests/ChangeLog:42
> > +        * http/tests/w3c/resources/testharness.js:
> 
> Has this change been made to testharness.js upstream in the W3C?

I'll have to do some research here to see if they have a newer version.

> > LayoutTests/http/tests/misc/acid3.html:601
> > +// COMMENTED OUT FOR 2011 UPDATE - we may want to merge RangeException and DOMException
> 
> Has this change been made to acid3 upstream?

I updated this part based on the upstream version.
Comment 9 Erik Arvidsson 2012-11-21 07:52:18 PST
I split the updates to Acid3 into a bug 102943
Comment 10 Erik Arvidsson 2012-12-03 09:09:31 PST
Comment on attachment 175253 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=175253&action=review

>>> Source/WebCore/dom/RangeException.idl:21
>>> +// web. We keep this for the ObjC bindings.
>> 
>> Should we add a #if for LANGUAGE_OBJECTIVEC to have the compiler enforce this statement?
> 
> Good idea.

If I wrap this in an #if then the non ObjC bindings complain that the file is empty.
Comment 11 Erik Arvidsson 2012-12-03 09:32:25 PST
Created attachment 177272 [details]
Patch
Comment 12 Erik Arvidsson 2012-12-03 09:38:58 PST
The new patch does not modify Acid3 tests nor the w3c test harness.
Comment 13 Erik Arvidsson 2012-12-03 10:57:27 PST
Created attachment 177287 [details]
Patch
Comment 14 Erik Arvidsson 2012-12-05 07:50:26 PST
Can anyone review this?
Comment 15 Ryosuke Niwa 2012-12-05 10:48:16 PST
Has this been announced on webkit-dev yet?
http://trac.webkit.org/wiki/DeprecatingFeatures
Comment 16 Erik Arvidsson 2012-12-11 11:22:55 PST
I'm not too happy about this. It keeps the cpp/h/idl files around just for the ObjectiveC bindings. Maybe we should just add bindings/objc/DOMRangeException.mm and remove all other traces of this?
Comment 17 Darin Adler 2012-12-11 11:46:13 PST
(In reply to comment #16)
> Maybe we should just add bindings/objc/DOMRangeException.mm and remove all other traces of this?

Sounds like it might work. It’s more than just the .mm file, but I agree that we don’t need to auto-generate it.
Comment 18 Anders Carlsson 2016-08-04 14:19:03 PDT
Looks like this has been done already.