Summary: | DOM Selection: collapse() and selectAllChildren() should throw WRONG_DOCUMENT_ERR if node belongs to other document | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED LATER | ||||||
Severity: | Normal | CC: | abarth, ap, eric, webkit.review.bot | ||||
Priority: | P2 | Keywords: | HTML5 | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | 45104 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Andreas Kling
2010-08-25 02:37:04 PDT
Created attachment 65387 [details]
Proposed patch
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. (In reply to comment #2) > Do these raise an exception in Firefox? No, only IE9 implements this behavior so far AFAIK. 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 :-(
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 Committed r66655: <http://trac.webkit.org/changeset/66655> http://trac.webkit.org/changeset/66655 might have broken Leopard Intel Release (Tests) Reopening due to editing test breakage. What is the status of this bug? The patch been rolled out, correct? See also: https://bugzilla.mozilla.org/show_bug.cgi?id=418755. (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. |