Bug 44595 – DOM Selection: collapse() and selectAllChildren() should throw WRONG_DOCUMENT_ERR if node belongs to other document

• Component: DOM
• Status: RESOLVED LATER

[Andreas Kling]

Selection.collapse() and Selection.selectAllChildren() should throw WRONG_DOCUMENT_ERR if the input node doesn't belong to the same document at the Selection.

This would cause us to pass the following test on IE Test Center:
http://samples.msdn.microsoft.com/ietestcenter/html5/selection/selection_wrongDocErr.htm

Spec links:
http://www.whatwg.org/specs/web-apps/current-work/#dom-selection-collapse
http://www.whatwg.org/specs/web-apps/current-work/#dom-selection-selectallchildren

[Andreas Kling]

Created attachment 65387 [details]
Proposed patch

[Alexey Proskuryakov]

Do these raise an exception in Firefox?

Raising exceptions where we didn't use to can be a significant compatibility issue, because if any page depended on the old behavior, it often stops working completely.

[Andreas Kling]

(In reply to comment #2)
> Do these raise an exception in Firefox?

No, only IE9 implements this behavior so far AFAIK.

[Antonio Gomes]

Comment on attachment 65387 [details]
Proposed patch

Gerally it also looks good to me. We need to find out the impact of such a change in existing web contents, witch is hard :-(

[Darin Adler]

Comment on attachment 65387 [details]
Proposed patch

> +    const Document* selectionDocument = m_frame ? m_frame->document() : 0;
> +    const Document* nodeDocument = node ? node->document() : 0;

The const here does no good.

> +    const Document* selectionDocument = m_frame ? m_frame->document() : 0;

Same here.

I’d like this even better if the regression test covered the null frame and null node cases. I can't tell if it does.

r=me

[Andreas Kling]

Committed r66655: <http://trac.webkit.org/changeset/66655>

[WebKit Review Bot]

http://trac.webkit.org/changeset/66655 might have broken Leopard Intel Release (Tests)

[Andreas Kling]

Reopening due to editing test breakage.

[Alexey Proskuryakov]

What is the status of this bug? The patch been rolled out, correct?

See also: https://bugzilla.mozilla.org/show_bug.cgi?id=418755.

[Andreas Kling]

(In reply to comment #9)
> What is the status of this bug? The patch been rolled out, correct?
> 
> See also: https://bugzilla.mozilla.org/show_bug.cgi?id=418755.

It was rolled out indeed (with r66661)

Interesting link, I'll close this. We can revisit the issue at a later time.