Closed Bug 636512 Opened 13 years ago Closed 13 years ago

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

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla8

People

(Reporter: ayg, Assigned: vishnus)

References

Details

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

Attachments

(3 files)

will change DOMString to nsISelection
763 bytes, patch
Details | Diff | Splinter Review
Changed implementation :)
2.40 KB, patch
bzbarsky
: review-
Details | Diff | Splinter Review
Corrected the build issue, changed the UUID
3.19 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
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: 13 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.
Keywords: dev-doc-needed → dev-doc-complete

    
    

    
  
(In reply to :Ms2ger from comment #21)
> https://developer.mozilla.org/en/XPCOM_Interface_Reference/
> nsIDOMNSHTMLDocument gets it wrong

Fixed.
Keywords: dev-doc-needed → dev-doc-complete

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: