Closed Bug 948475 Opened 9 years ago Closed 9 years ago

Implement HTMLInputElement.select() for <input type=number>, contrary to the HTML5 spec

Categories

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

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla29
Tracking Flags:
Tracking Status
firefox28 --- fixed
firefox29 --- fixed

People

(Reporter: jwatt, Assigned: jwatt)

References

Details

Attachments

(1 file, 2 obsolete files)

patch
4.91 KB, patch
smaug
: review+
lsblakk
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
I didn't implement HTMLInputElement.select() for <input type=number> because the spec says it doesn't apply:

http://www.whatwg.org/specs/web-apps/current-work/multipage/states-of-the-type-attribute.html#number-state-%28type=number%29

Turns out that Chrome implements it though, and pdf.js relies on it working:

https://github.com/mozilla/pdf.js/pull/4020#issuecomment-30243390

I think rather than select() just doing nothing, it seems reasonable to forward the call on to its anonymous text control grandchild.
Attached patch patch (obsolete) — — Splinter Review 

        Attachment #8345341 -
        Flags: review?(bugs)

    
    

    
  
Err, disregard the viewer.css change - I was testing pdf.js locally. Removed from my version of the patch.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch (obsolete)
        — 
          — Splinter Review
      

    


  

        Attachment #8345341 -
        Attachment is obsolete: true

        Attachment #8345341 -
        Flags: review?(bugs)

        Attachment #8345346 -
        Flags: review?(bugs)

    
  
Blocks: 948433

    
    

    
  
Can you please bring this up on the whatwg list so that we can change the spec?  Thanks!

    
    

    
  
I'm very late starting my PTO. I'll let someone else do that or it can wait until January.

    
    

    
  
Comment on attachment 8345346 [details] [diff] [review]
patch

Isn't this missing number-selected.html  and number-selected-ref.html

        Attachment #8345346 -
        Flags: review?(bugs) → review-

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch
        — 
          — Splinter Review
      

    


  

        Attachment #8345346 -
        Attachment is obsolete: true

        Attachment #8345521 -
        Flags: review?(bugs)

    
    

    
  
Oops, sorry. Pushed this by mistake when qfinishing and pushing bug 946184 since this was also applied. I'll leave it in since I'm sure you're fine with this modulo seeing the tests. Let me know if you want any changes.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fa744c4a8948

        Attachment #8345521 -
        Flags: review?(bugs) → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/fa744c4a8948
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla29

    
  
Blocks: 344616

    
    

    
  
Comment on attachment 8345521 [details] [diff] [review]
patch

Bug caused by (feature/regressing bug #): 344616 and its dependencies
User impact if declined: for example, the page number field in pdf.js doesn't select
Testing completed (on m-c, etc.): landed m-i, merged to m-c
Risk to taking this patch (and alternatives if risky): very low
String or IDL/UUID changes made by this patch: none

We're early in the cycle so hopefully this is fine to uplift.

        Attachment #8345521 -
        Flags: approval-mozilla-aurora?

        Attachment #8345521 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
  

          status-firefox28:
          --- → fixed
Target Milestone: mozilla29 → mozilla28

          status-firefox29:
          --- → fixed
Target Milestone: mozilla28 → mozilla29

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