Closed Bug 636512 Opened 10 years ago Closed 9 years ago

document.getSelection() should return the same as getSelection(), not stringification

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla8

People

(Reporter: ayg, Assigned: vishnus)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [good first bug])

Attachments

(3 files)

Per spec:

"For historical reasons, the getSelection() method on the Document interface must return the same Selection object as its defaultView's getSelection() method."
http://html5.org/specs/dom-range.html#dom-document-getselection

This is how WebKit (Chrome 10 dev) and IE 9 RC implement it.  Firefox 4b11 and Opera 11 return a string.  Test case:

data:text/html,<!doctype html>
<script>alert(typeof document.getSelection())</script>

Alerts "string" in Firefox and Opera, "object" in IE and WebKit.  The behavior of the original API was actually to return a string (see bug 32481), but the specced behavior seems simpler, more convenient, and more intuitive, unless it will cause compat problems.  If Gecko doesn't want to change this, the spec should probably change.
I think making this change should be fine (post-2.0).

Looks like just changing the idl and removing the stringification might be good enough.

Should we take out the warning too?
Component: DOM: Traversal-Range → DOM: Core & HTML
QA Contact: traversal-range → general
Whiteboard: [good first bug]
I don't see any reason for the warning.
toolkit/components/console/hudservice/tests/browser/browser_webconsole_bug_595934_message_categories.js includes a test for the warning that will need to be removed.
Fun gotcha I found that this behavior causes: if you use "getSelection()" to retrieve the Selection object (instead of "window.getSelection()"), it will work everywhere except in event handlers, where it will mean document.getSelection() instead of window.getSelection().  I just hit that in a test-case, and I'd have been scratching my head for an awfully long time over it if I didn't know about this quirk.
This is my first patch :) thanks to Ms2ger for the guidelines :) hope this is the right one :)
Implementation part changed. Again thanks to Ms2ger :)
Attachment #544270 - Flags: review+
Attachment #544270 - Flags: review+ → review?(bzbarsky)
Comment on attachment 544270 [details] [diff] [review]
Changed implementation :)

This needs a checkin comment and a User or From line, but more importantly needs to rev the nsIDOMHTMLDocument iid.
Attachment #544270 - Flags: review?(bzbarsky) → review-
Attachment #544521 - Flags: review?(bzbarsky)
Comment on attachment 544521 [details] [diff] [review]
Corrected the build issue, changed the UUID

r=me.  Thank you for the patch!
Attachment #544521 - Flags: review?(bzbarsky) → review+
so is that it? :) :)
That's it in terms of things you have to do.  The patch still needs to be landed, but I or one of the other folks managing mozilla-inbound will take care of that.
oh ok :) thank you. :) and thank you Ms2ger :)
Ah, indeed.  Good catch.  I'll remove that test before pushing this.  I checked the other consumers, and they're all working with windows, not documents.
http://hg.mozilla.org/integration/mozilla-inbound/rev/764fbee6b335
Assignee: nobody → onlinevishnu
Flags: in-testsuite?
Target Milestone: --- → mozilla8
http://hg.mozilla.org/mozilla-central/rev/764fbee6b335
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
Depends on: 692412
(In reply to Aryeh Gregor from comment #2)
> I don't see any reason for the warning.

Why not? Could have saved at least one bug report and probably a lot of unreported Developer's trouble.
IIRC, the warning previously said the feature was deprecated or similar, which isn't true.  So I don't see any reason for that warning.  It might be useful to have a warning for a couple of years saying that different browsers treat the attribute differently and you should use window.getSelection() instead if you want your code to work cross-browser.  But it seems like you could add warnings like that to lots of features, and usually Gecko doesn't, does it?
Depends on: 690997
This doc was already updated, and the change is listed on Firefox 8 for developers.
Duplicate of this bug: 697184
(In reply to :Ms2ger from comment #21)
> https://developer.mozilla.org/en/XPCOM_Interface_Reference/
> nsIDOMNSHTMLDocument gets it wrong

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