Closed Bug 718711 Opened 8 years ago Closed 2 years ago

getSelection() should exist on XMLDocument

Categories

(Core :: DOM: Selection, defect, minor)

x86
Linux
defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ayg, Assigned: ayg)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

DOM4 defines XMLDocument as extending Document:

http://dvcs.w3.org/hg/domcore/raw-file/tip/dom-core.html#xmldocument

The editing spec defines getSelection as an operation on the Document interface (scroll up slightly for the IDL):

http://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#dom-document-getselection

(In fact, HTML now requires that window.HTMLDocument === window.Document: http://www.whatwg.org/specs/web-apps/current-work/#htmldocument)

So one implication of all this is that "getSelection" in XMLDocument.prototype should be true, but in Firefox 12.0a1 (2012-01-12) it's not:

data:text/html,<!doctype html>
<script>alert("getSelection" in XMLDocument.prototype)</script>

This alerts false in Firefox 12.0a1 and Opera Next 12.00 alpha, and true in Chrome 17 dev.  In IE9 it throws, because window.XMLDocument is undefined.  The specs agree with WebKit.  This causes a test failure in the Selection conformance tests, test name "XML document with no browsing context", "assert_true: XML document must have getSelection() expected true got false":

http://dvcs.w3.org/hg/editing/raw-file/tip/selecttest/getSelection.html
Depends on: 512688
All that needs to happen is to move getSelection from HTMLDocument to Document in our IDL, right?
Sounds likely.  Per current HTML, there should actually be no HTMLDocument interface -- window.HTMLDocument should just be an alias of window.Document.
No longer depends on: 512688
Depends on: 897815
Comment on attachment 8895849 [details]
Bug 718711 - getSelection() should exist for XML documents;

I don't have permission to review this.
Attachment #8895849 - Flags: review?(masayuki) → review?(bugs)
Comment on attachment 8895849 [details]
Bug 718711 - getSelection() should exist for XML documents;

https://reviewboard.mozilla.org/r/167140/#review172758

::: dom/base/nsDocument.cpp:13536
(Diff revision 1)
>  }
> +
> +Selection*
> +nsIDocument::GetSelection(ErrorResult& aRv)
> +{
> +  nsCOMPtr<nsPIDOMWindowInner> window = do_QueryInterface(GetScopeObject());

getSelection should return null when document isn't associated with a browsing context, but you seem always return selection if there just is scope object.
Since you only moved this code, I guess it is ok. But could you file a follow to make
document.implementation.createHTMLDocument().getSelection(); to return null.

Or feel free to fix the issue in this bug too.
Instead of using GetScopeObject() GetInnerWindow() would be more correct.
Attachment #8895849 - Flags: review?(bugs) → review+
Assignee: nobody → ayg
Status: NEW → ASSIGNED
Comment on attachment 8896718 [details]
Bug 718711 - Don't return selections for documents that don't have them;

https://reviewboard.mozilla.org/r/168012/#review173608
Attachment #8896718 - Flags: review?(bugs) → review+
Pushed by ayg@aryeh.name:
https://hg.mozilla.org/integration/autoland/rev/320b085f1591
getSelection() should exist for XML documents; r=smaug
https://hg.mozilla.org/integration/autoland/rev/91c0a4f99001
Don't return selections for documents that don't have them; r=smaug
https://hg.mozilla.org/mozilla-central/rev/320b085f1591
https://hg.mozilla.org/mozilla-central/rev/91c0a4f99001
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
I've added a note to the Fx57 rel notes covering this:

https://developer.mozilla.org/en-US/Firefox/Releases/57#DOM

(No docs updates are needed, as getSelection was already put on the Document (and Window) interface as per the spec)
You need to log in before you can comment on or make changes to this bug.