933539 - Use parseFloat instead of parseInt for the titlebar calculations in browser.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

The styles that are being read for the titlebar calculations in browser.js have the potential to be defined as real numbers, so we should use parseFloat instead of parseInt to read back the computed styles.

To fix this bug, you'll need to clone the UX branch which is located at https://hg.mozilla.org/projects/ux. The file in question is located at browser/base/content/browser.js and you'll want to look at the lines around http://mxr.mozilla.org/projects-central/source/ux/browser/base/content/browser.js#4307

Comment 1

[Tarashish Mishra [:sunu]]

I want to work on this one. Please assign it to me.

Comment 2

[:Gijs (he/him)]

(In reply to Tarashish Mishra from comment #1)
> I want to work on this one. Please assign it to me.

All yours!

Comment 3

[Tarashish Mishra [:sunu]]

Attachment: name.patch

Hi! I have attached a patch. Don't know if that's all I have to do for this bug. Please have a look and let me know if I need to fix anything else.

Comment 4

[:Gijs (he/him)]

Comment on attachment 825893 [details] [diff] [review]
name.patch

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

Sorry, but this is not enough. You'll need to adjust all the lines in this function that use parseInt to read style values to use parseFloat.

Comment 5

[Tarashish Mishra [:sunu]]

Attachment: second.patch

Thanks for the feedback. I have updated the patch. Please have a look.

Comment 6

[:Gijs (he/him)]

Comment on attachment 825895 [details] [diff] [review]
second.patch

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

r=me without the bits in the gFormSubmitObserver, which shouldn't be changed. I did say "in this function"... Anyway, I'll land this with the nits fixed.

::: browser/base/content/browser.js
@@ +721,5 @@
>        let utils = win.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>                       .getInterface(Components.interfaces.nsIDOMWindowUtils);
>  
>        if (style.direction == 'rtl') {
> +        offset = parseFloat(style.paddingRight) + parseFloat(style.borderRightWidth);

These two lines are in a different function, so we should probably leave them alone.

Comment 7

[Tarashish Mishra [:sunu]]

Thanks :)

Comment 8

[:Gijs (he/him)]

https://hg.mozilla.org/projects/ux/rev/af0c1944815c

Comment 9

[:Gijs (he/him)]

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