Last Comment Bug 891340 - Make Range.collapse optional and default to false
: Make Range.collapse optional and default to false
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla25
Assigned To: Anne (:annevk)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-09 06:31 PDT by Anne (:annevk)
Modified: 2013-08-08 07:25 PDT (History)
5 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
attempt 1 (413 bytes, patch)
2013-07-09 06:55 PDT, Anne (:annevk)
no flags Details | Diff | Splinter Review
attempt 2 (996 bytes, patch)
2013-07-09 12:38 PDT, Anne (:annevk)
amarchesini: review+
Details | Diff | Splinter Review
attempt 2.1 [r=baku] (1.84 KB, patch)
2013-07-09 15:25 PDT, Anne (:annevk)
annevk: review+
Details | Diff | Splinter Review

Description Anne (:annevk) 2013-07-09 06:31:35 PDT
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) 2013-07-09 06:55:32 PDT
Created attachment 772634 [details] [diff] [review]
attempt 1
Comment 2 Andrea Marchesini [:baku] 2013-07-09 07:55:16 PDT
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 (⌚ UTC+1/+2) 2013-07-09 09:48:28 PDT
Needs a test!
Comment 4 Anne (:annevk) 2013-07-09 10:09:54 PDT
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) 2013-07-09 10:25:18 PDT
https://github.com/w3c/web-platform-tests/pull/243
Comment 6 Andrea Marchesini [:baku] 2013-07-09 12:26:21 PDT
Comment on attachment 772634 [details] [diff] [review]
attempt 1

There is something orange on try.
Comment 7 Anne (:annevk) 2013-07-09 12:38:18 PDT
Created attachment 772817 [details] [diff] [review]
attempt 2

Bah.
Comment 8 Anne (:annevk) 2013-07-09 12:50:07 PDT
https://github.com/w3c/web-platform-tests/pull/244 is to fix comment 7 upstream.
Comment 9 Andrea Marchesini [:baku] 2013-07-09 15:04:00 PDT
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) 2013-07-09 15:25:18 PDT
Created attachment 772934 [details] [diff] [review]
attempt 2.1 [r=baku]
Comment 11 Andrea Marchesini [:baku] 2013-07-09 15:30:27 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/3537ffb67d4b
Comment 12 Ryan VanderMeulen [:RyanVM] 2013-07-10 11:20:28 PDT
https://hg.mozilla.org/mozilla-central/rev/3537ffb67d4b
Comment 13 Jean-Yves Perrier [:teoli] 2013-07-30 09:26:44 PDT
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] 2013-08-07 15:32:48 PDT
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] (vacation Aug 25-28) 2013-08-08 02:28:50 PDT
You mean the WebIDL parser for tests, right? Gecko's WebIDL parser sure supports optional.
Comment 16 Masatoshi Kimura [:emk] 2013-08-08 07:25:34 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.