Bug 171777 - Align our IDL files with the latest DOM specification
Summary: Align our IDL files with the latest DOM specification
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-06 15:22 PDT by Chris Dumez
Modified: 2017-05-06 20:48 PDT (History)
12 users (show)

See Also:


Attachments
WIP Patch (36.51 KB, patch)
2017-05-06 15:23 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (37.23 KB, patch)
2017-05-06 15:41 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (49.48 KB, patch)
2017-05-06 16:38 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews114 for mac-elcapitan (2.01 MB, application/zip)
2017-05-06 18:43 PDT, Build Bot
no flags Details
Patch (49.46 KB, patch)
2017-05-06 19:40 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (49.46 KB, patch)
2017-05-06 19:41 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2017-05-06 15:22:43 PDT
Align our IDL files with the latest DOM specification:
- https://dom.spec.whatwg.org
Comment 1 Chris Dumez 2017-05-06 15:23:21 PDT
Created attachment 309298 [details]
WIP Patch
Comment 2 Chris Dumez 2017-05-06 15:41:49 PDT
Created attachment 309299 [details]
WIP Patch
Comment 3 Chris Dumez 2017-05-06 16:38:15 PDT
Created attachment 309307 [details]
Patch
Comment 4 Sam Weinig 2017-05-06 18:41:43 PDT
Comment on attachment 309307 [details]
Patch

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

v.nice r=me

> Source/WebCore/dom/Document.idl:175
> +    // Non standard.
> +    Range caretRangeFromPoint(optional long x = 0, optional long y = 0);

I would note it has been superseded by caretPositionFromPoint, which we don't yet implement.

> Source/WebCore/dom/Range.idl:64
> +    DOMString toString(); // FIXME: Should be stringifier once we support it.

I believe we do support stringifier.  It's used in a number of other IDLs.

> Source/WebCore/dom/Range.idl:73
> +    // Non Standard API.

Might want to standardize on how you write "non standard".  In other places you left off the API and made standard all lowercase.

> Source/WebCore/dom/Text.idl:25
> +    [NewObject, MayThrowException] Text splitText(unsigned long offset);

What happens when offset is 0? Does it really still return a new object?
Comment 5 Build Bot 2017-05-06 18:43:32 PDT
Comment on attachment 309307 [details]
Patch

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

New failing tests:
fast/dom/Range/split-text-in-range.html
Comment 6 Build Bot 2017-05-06 18:43:33 PDT
Created attachment 309311 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 7 Chris Dumez 2017-05-06 19:30:26 PDT
Comment on attachment 309307 [details]
Patch

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

>> Source/WebCore/dom/Document.idl:175
>> +    Range caretRangeFromPoint(optional long x = 0, optional long y = 0);
> 
> I would note it has been superseded by caretPositionFromPoint, which we don't yet implement.

Ok. I wasn't aware. Thanks for the info. I'll add the note.

>> Source/WebCore/dom/Range.idl:64
>> +    DOMString toString(); // FIXME: Should be stringifier once we support it.
> 
> I believe we do support stringifier.  It's used in a number of other IDLs.

No. I added support a while back for stringifier, but only on attributes. We do not support yet:
stringifier;

>> Source/WebCore/dom/Range.idl:73
>> +    // Non Standard API.
> 
> Might want to standardize on how you write "non standard".  In other places you left off the API and made standard all lowercase.

Ok, will drop API.

>> Source/WebCore/dom/Text.idl:25
>> +    [NewObject, MayThrowException] Text splitText(unsigned long offset);
> 
> What happens when offset is 0? Does it really still return a new object?

Yes, we always return a new object, as per the spec:
- https://dom.spec.whatwg.org/#concept-text-split

That said, there is a test that is failing so I'll have to drop the NewObject anyway. The reason the test is failing is because we fire the DOMNodeInserted event synchronously before returning and therefore, the JS may have already created a wrapper for the Text node by the type we return, in the case where they have a DOMNodeInserted listener (see LayoutTests/fast/dom/Range/split-text-in-range.html).
Comment 8 Chris Dumez 2017-05-06 19:40:01 PDT
Created attachment 309313 [details]
Patch
Comment 9 Chris Dumez 2017-05-06 19:41:02 PDT
Created attachment 309314 [details]
Patch
Comment 10 WebKit Commit Bot 2017-05-06 20:48:32 PDT
Comment on attachment 309314 [details]
Patch

Clearing flags on attachment: 309314

Committed r216339: <http://trac.webkit.org/changeset/216339>
Comment 11 WebKit Commit Bot 2017-05-06 20:48:34 PDT
All reviewed patches have been landed.  Closing bug.