948433 - PDF.js needs updating to take account of the new <input type=number> support in Firefox 28

Status: VERIFIED FIXED

(Firefox :: PDF Viewer, defect)

Description

[Jonathan Watt [:jwatt]]

The pref to enable <input type=number> has been flipped for Firefox 28. It seems that PDF.js needs to be updated for that in order to hide the spinner. The mechanism that we've provided to do that is to set |-moz-appearance:textfield| for the <input type=number> instead of the default |-moz-appearance:number-input| that it has.

Comment 1

[Jonathan Watt [:jwatt]]

Pull request at: https://github.com/mozilla/pdf.js/pull/4020

What's the procedure to get this uplifted for aurora?

Comment 2

[Jonathan Watt [:jwatt]]

Note that bug 948475 and bug 946184 are also needed to get pdf.js back to looking and behaving as it did/was before the <input type=number> support was enabled.

Comment 3

[Brendan Dahl [:bdahl]]

Uplift procedure https://github.com/mozilla/pdf.js/wiki/Updating-pdf.js-on-Mozilla-Central#if-uplift-is-needed

Comment 4

[Jonathan Watt [:jwatt]]

So basically, make a patch from the git commit, get approval, land.

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: patch

Did I do this right?

Comment 6

[Brendan Dahl [:bdahl]]

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

Review of attachment 8345437 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry I should add to the instructions to remove all the version number changes. It should just be your one line change.

Comment 7

[Brendan Dahl [:bdahl]]

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

Review of attachment 8345437 [details] [diff] [review]:
-----------------------------------------------------------------

r+ pending removal of everything else.  (guess there was no point to those steps for this one liner)

Comment 8

[Jonathan Watt [:jwatt]]

Thanks for that. Nice not to have to re-request review for something trivial. :)

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

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fb0fa2c21392

Comment 10

[Jonathan Watt [:jwatt]]

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A (or bug 344616 if you need one to blame)
User impact if declined: pdf.js UI will be broken (have an ugly spinner in the page number field)
Testing completed (on m-c, etc.): landed m-i, merged to m-c
Risk to taking this patch (and alternatives if risky): low and early
String or IDL/UUID changes made by this patch: none

We're early in the cycle so hopefully this is fine to uplift. Probably needed to keep <input type=number> on in v28.

Comment 11

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c20510b372a6

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Let's leave this open until the updated pdf.js hits m-c as well.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

This already hit m-c, so nevermind. I'll still mark the dependency on the next official "update pdf.js" bug.

Comment 14

[u279076]

Are there any known testcases we can use to verify this is fixed?

Comment 15

[Jonathan Watt [:jwatt]]

Open any PDF with Firefox's built-in PDF viewer and if the page number field in the UI doesn't have spin buttons then we're good. If this wasn't fixed we'd be getting more bug reports though, so I think we're good.

Comment 16

[Ioana (away)]

Verified as fixed on the latest Firefox 28.0a2 and Firefox 29.0a1 (January 26th and 27th builds), on Windows 7 64bit, Mac OS X 10.7.5 and Ubuntu 13.04 32bit (reproduced the issue on older Aurora and Nightly builds before verifying the fix).