Closed Bug 891340 Opened 8 years ago Closed 8 years ago

Make Range.collapse optional and default to false

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: annevk, Assigned: annevk)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 2 obsolete files)

This should be a fairly trivial IDL change and would be consistent with other boolean arguments we've made optional. http://dom.spec.whatwg.org/#range
Attached patch attempt 1 (obsolete) — Splinter Review
Assignee: mounir → annevk
Attachment #772634 - Flags: review?(amarchesini)
Comment on attachment 772634 [details] [diff] [review]
attempt 1

Review of attachment 772634 [details] [diff] [review]:
-----------------------------------------------------------------

Congrats! Your first r+!
I sent your patch to try. If it's green I land it.

https://tbpl.mozilla.org/?tree=Try&rev=f7e9f24d8f49
Attachment #772634 - Flags: review?(amarchesini) → review+
Needs a test!
A test should be added to dom/ranges/Range-collapse.html in https://github.com/w3c/web-platform-tests It seems that can be done separately.
Comment on attachment 772634 [details] [diff] [review]
attempt 1

There is something orange on try.
Attachment #772634 - Flags: review+
Attached patch attempt 2 (obsolete) — Splinter Review
Bah.
Attachment #772634 - Attachment is obsolete: true
Attachment #772817 - Flags: review?(amarchesini)
Comment on attachment 772817 [details] [diff] [review]
attempt 2

Review of attachment 772817 [details] [diff] [review]:
-----------------------------------------------------------------

https://tbpl.mozilla.org/?tree=Try&rev=c11f86a3a8eb
Attachment #772817 - Flags: review?(amarchesini) → review+
Keywords: checkin-needed
Attachment #772817 - Attachment is obsolete: true
Attachment #772934 - Flags: review+
Attachment #772934 - Attachment description: attempt 2.1 → attempt 2.1 [r=baku]
https://hg.mozilla.org/mozilla-central/rev/3537ffb67d4b
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
The current WebIDL parser doesn't support optional arguments.
https://tbpl.mozilla.org/php/getParsedLog.php?id=26263236&tree=Mozilla-Inbound#error1
Even worse, the current testharness doesn't catch the error. So the test is effectively disabled.
https://bugzilla.mozilla.org/show_bug.cgi?id=885107#c40
You mean the WebIDL parser for tests, right? Gecko's WebIDL parser sure supports optional.
(In reply to Olli Pettay [:smaug] from comment #15)
> You mean the WebIDL parser for tests, right? Gecko's WebIDL parser sure
> supports optional.

Yes, I meant dom/imptests/WebIDLParser.js.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.