Closed
Bug 737565
Opened 13 years ago
Closed 13 years ago
Range.comparePoint() should throw exceptions per spec
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: ayg, Assigned: ayg)
Details
Attachments
(1 file)
|
6.02 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
Spec:
"""
3. If node is a doctype, throw an "InvalidNodeTypeError" exception and terminate these steps.
4. If offset is greater than node's length, throw an "IndexSizeError" exception and terminate these steps.
"""
http://dvcs.w3.org/hg/domcore/raw-file/tip/dom-core.html#dom-range-comparepoint
Gecko doesn't throw in either of these cases. I mandated throwing here in the spec to match Chrome, as noted in the comments in the spec source:
<!-- Firefox 9.0a2 doesn't throw. It ignores the offset and returns true or
false depending on whether the doctype itself is in the range. This makes
some sense, but it doesn't match how other Range APIs handle doctypes, and
having the second argument mandatory but ignored is just weird. Thus I go
with Chrome 16 dev, although I can see the merit in how Gecko works here. -->
<!-- Firefox 9.0a2 doesn't throw. It seems to return true if the node is
completely contained in the range, like with selectNode(), and false otherwise
- even if all boundary points in the node are contained in the range, like
with selectNodeContents(). This is weird, and inconsistent with other Range
APIs, so I go with Chrome 16 dev. This is probably an authoring bug, so it's
best to throw anyway. -->
Test-case for doctypes:
data:text/html,<!doctype html>
<script>
var range = document.createRange();
document.documentElement.textContent = "didn't throw";
try { range.comparePoint(document.doctype, 0) }
catch(e) { document.documentElement.textContent = e }
</script>
Gecko and Opera Next 12.00 alpha are "didn't throw". Chrome 19 dev is "Error: INVALID_NODE_TYPE_ERR: DOM Range Exception 2". IE10 Developer Preview doesn't support the method at all.
Test-case for excessive offset:
data:text/html,<!doctype html>
<script>
var range = document.createRange();
document.documentElement.textContent = "didn't throw";
try { range.comparePoint(document, 75) }
catch(e) { document.documentElement.textContent = e }
</script>
Gecko/Opera are "didn't throw", Chrome is "Error: INDEX_SIZE_ERR: DOM Exception 1".
This is causing test failures in <http://w3c-test.org/webapps/DOMCore/tests/approved/Range-comparePoint.html>.
|
||
Comment 1•13 years ago
|
||
Web compat impact? Moving from "don't throw" to "throw" has a tendency to cause web compat problems, even if other browsers are in the "throw" bucket (due to browser sniffing)....
|
Assignee | |
Comment 2•13 years ago
|
||
Well, here's a patch that fixes the bug, if we want to fix it. If this is WONTFIX, I'll change the spec to match Gecko instead, although IMO Gecko doesn't make sense here. But I'm guessing there's not much web compat risk. It would have to be Gecko-specific code that does something that makes little sense to start with.
|
|
Assignee | |
Updated•13 years ago
|
Whiteboard: [autoland-try:-u all]
|
||
Updated•13 years ago
|
Whiteboard: [autoland-try:-u all] → [autoland-in-queue]
|
|
||
Comment 3•13 years ago
|
||
Autoland Patchset:
Patches: 607674
Branch: mozilla-central => try
Destination: http://hg.mozilla.org/try/pushloghtml?changeset=f3885a8a6a68
Try run started, revision f3885a8a6a68. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=f3885a8a6a68
|
||
Comment 4•13 years ago
|
||
Comment on attachment 607674 [details] [diff] [review]
Patch v1
We should try this, and change the spec if something breaks.
Attachment #607674 -
Flags: review?(bugs) → review+
|
|
||
Comment 5•13 years ago
|
||
Try run for f3885a8a6a68 is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=f3885a8a6a68
Results (out of 186 total builds):
exception: 4
success: 149
warnings: 19
failure: 13
other: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-f3885a8a6a68
Timed out after 12 hours without completing.
|
|
||
Updated•13 years ago
|
Whiteboard: [autoland-in-queue]
|
|
Assignee | |
Comment 6•13 years ago
|
||
Flags: in-testsuite+
OS: Linux → All
Hardware: x86 → All
Target Milestone: --- → mozilla14
|
||
Comment 7•13 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
|
||
Updated•12 years ago
|
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•