Make Range.collapse optional and default to false

RESOLVED FIXED in mozilla25

Status

()

defect
RESOLVED FIXED
6 years ago
4 months ago

People

(Reporter: annevk, Assigned: annevk)

Tracking

({dev-doc-complete})

unspecified
mozilla25
x86
macOS
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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
Posted 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+
Posted 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: 6 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.