891340 - Make Range.collapse optional and default to false

Status: RESOLVED FIXED

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

Description

[Anne (:annevk)]

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

Comment 1

[Anne (:annevk)]

Attachment: attempt 1

Comment 2

[Andrea Marchesini [:baku]]

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

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Needs a test!

Comment 4

[Anne (:annevk)]

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 5

[Anne (:annevk)]

https://github.com/w3c/web-platform-tests/pull/243

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 772634 [details] [diff] [review]
attempt 1

There is something orange on try.

Comment 7

[Anne (:annevk)]

Attachment: attempt 2

Bah.

Comment 8

[Anne (:annevk)]

https://github.com/w3c/web-platform-tests/pull/244 is to fix comment 7 upstream.

Comment 9

[Andrea Marchesini [:baku]]

Comment on attachment 772817 [details] [diff] [review]
attempt 2

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

https://tbpl.mozilla.org/?tree=Try&rev=c11f86a3a8eb

Comment 10

[Anne (:annevk)]

Attachment: attempt 2.1 [r=baku]

Comment 11

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3537ffb67d4b

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3537ffb67d4b

Comment 13

[Jean-Yves Perrier [:teoli]]

Updated:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/25
and
https://developer.mozilla.org/en-US/docs/Web/API/range.collapse

Btw: it looks like IE (IE9-11) is defaulting to true (and the spec say false) See: http://msdn.microsoft.com/en-us/library/ie/ff975439%28v=vs.85%29.aspx

Comment 14

[Masatoshi Kimura [:emk]]

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

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

You mean the WebIDL parser for tests, right? Gecko's WebIDL parser sure supports optional.

Comment 16

[Masatoshi Kimura [:emk]]

(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.