1042619 - In Inspector, box model view, use the sign × instead of the letter x

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Nicolas Barbulesco]

Firefox 31, on Win 7.

The box model view of the Inspector is (well) described here : 
https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector#Box_model_view

In the Inspector, we have sizes of boxes like this : 
"240x125" with the letter x of the latin alphabet

I think it would be better to have the sizes like this : 
"240×125" with the multiplication sign ×

It would be more correct. The intented meaning here is the sign "times". The letter x has nothing to do here, it just landed here by a typographic accident of history.

The similar bug 677912 in Firefox has been fixed. 
Correcting this issue in the Inspector of Firefox would be consistent. 

Google Chrome's Inspector gives sizes of boxes like this : 
"274px × 105px"

Thank you.

Comment 1

[Aaron Raimist [:aaronraimist]]

Attachment: bug1042619_timessign.diff

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8540478 [details] [diff] [review]
bug1042619_timessign.diff

Redirecting to bgrins.

Comment 3

[Brian Grinstead [:bgrins]]

Comment on attachment 8540478 [details] [diff] [review]
bug1042619_timessign.diff

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

Looks good.  I'd like to add a shared.properties file and so we can localize strings like this (and other commonly used strings across tools), but I'll file a follow up bug for that.

Comment 4

[Brian Grinstead [:bgrins]]

https://hg.mozilla.org/integration/fx-team/rev/291e3a83a122

Comment 5

[Brian Grinstead [:bgrins]]

So apparently using the 'x' character in a commit message breaks builds (Bug 1121132).  Also, I suspect this may cause some test failures - I'll push this to try.

Backed out: https://hg.mozilla.org/integration/fx-team/rev/268dfa4925ec

Comment 6

[Brian Grinstead [:bgrins]]

There are some tests that are failing with this applied.  For instance in the layoutview there are things checking for 100x150, etc:

https://dxr.mozilla.org/mozilla-central/source/browser/devtools/layoutview/test/browser_layoutview.js#12
https://dxr.mozilla.org/mozilla-central/source/browser/devtools/layoutview/test/browser_layoutview_update-after-navigation.js#48

Can you update the tests to look for the × character instead?  You can run the tests locally to check with ./mach mochitest-devtools browser/devtools/layoutview/

Here is a try push, which will have full test results when it finishes: https://tbpl.mozilla.org/?tree=Try&rev=031dc11ff815

Comment 7

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #6)
> Here is a try push, which will have full test results when it finishes:
> https://tbpl.mozilla.org/?tree=Try&rev=031dc11ff815

There are also changes needed for the canvas debugger and responsive UI tests that can be seen in the link

Comment 8

[Aaron Raimist [:aaronraimist]]

Attachment: bug1042619_width×height_devtools_v2.diff

Comment 9

[Brian Grinstead [:bgrins]]

Comment on attachment 8550913 [details] [diff] [review]
bug1042619_width×height_devtools_v2.diff

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

Can we just use the symbol - × - throughout these files?  It will be a lot easier to read.

Comment 10

[Brian Grinstead [:bgrins]]

Also, I'm still seeing a failure in browser/devtools/netmonitor/test/browser_net_content-type.js and browser/devtools/markupview/test/browser_markupview_image_tooltip.js.  Relevant try push: https://tbpl.mozilla.org/?tree=Try&rev=72372b29ec5c

Comment 11

[Aaron Raimist [:aaronraimist]]

I can do that for everything except the responsive design view. In the responsive design view dropdown menu I get unknown character boxes. Do you know what I would need to change to make it work without using escape sequences?

http://i.imgur.com/k8ltCg8.png
http://i.imgur.com/YdNAwT1.png

Comment 12

[Brian Grinstead [:bgrins]]

(In reply to Aaron Raimist [:aaronraimist] from comment #11)
> I can do that for everything except the responsive design view. In the
> responsive design view dropdown menu I get unknown character boxes. Do you
> know what I would need to change to make it work without using escape
> sequences?

That's because the file is saved as charset=us-ascii while the other files are saved with charset=utf-8.  So, you'll need to save that file with UTF8 encoding.

Comment 13

[Brian Grinstead [:bgrins]]

(In reply to Brian Grinstead [:bgrins] from comment #12)
> (In reply to Aaron Raimist [:aaronraimist] from comment #11)
> > I can do that for everything except the responsive design view. In the
> > responsive design view dropdown menu I get unknown character boxes. Do you
> > know what I would need to change to make it work without using escape
> > sequences?
> 
> That's because the file is saved as charset=us-ascii while the other files
> are saved with charset=utf-8.  So, you'll need to save that file with UTF8
> encoding.

I was wrong about that - it still shows up this way even when resaving.  It may have to do with the charset on the browser xul file.  Either way, I'd say let's switch back to the escape code, but only for the 3 files in the original patch (canvasdebugger.js | view.js | responsivedesign.jsm).  Keep the tests using the symbol.

Comment 14

[Aaron Raimist [:aaronraimist]]

Attachment: bug1042619_width×height_devtools_v3.diff

I used × in the tests and the escape code in everything else.

Comment 15

[Brian Grinstead [:bgrins]]

Pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff11ceed1ff2

Comment 16

[Aaron Raimist [:aaronraimist]]

(In reply to Brian Grinstead (unavailable through 1-26) [:bgrins] from comment #15)
> Pushed to try:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff11ceed1ff2

Looks like the symbol doesn't work in the tests either:

 INFO TEST-UNEXPECTED-FAIL | browser/devtools/markupview/test/browser_markupview_image_tooltip.js | Tooltip label for [img.local] displays the right image size - Got 192 × 192, expected 192 Ã 192

Comment 17

[Brian Grinstead [:bgrins]]

(In reply to Aaron Raimist [:aaronraimist] from comment #16)
> (In reply to Brian Grinstead (unavailable through 1-26) [:bgrins] from
> comment #15)
> > Pushed to try:
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff11ceed1ff2
> 
> Looks like the symbol doesn't work in the tests either:
> 
>  INFO TEST-UNEXPECTED-FAIL |
> browser/devtools/markupview/test/browser_markupview_image_tooltip.js |
> Tooltip label for [img.local] displays the right image size - Got 192 × 192,
> expected 192 Ã 192

Darn, sorry about that.  Can you replace those instances with the escape code instead?

Comment 18

[Aaron Raimist [:aaronraimist]]

Attachment: bug1042619_width×height_devtools_v4.diff

Comment 19

[Brian Grinstead [:bgrins]]

Comment on attachment 8553466 [details] [diff] [review]
bug1042619_width×height_devtools_v4.diff

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

Try push looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c9e331dec924

Comment 20

[Brian Grinstead [:bgrins]]

The commit message will need to be changed before landing this to remove the unicode symbol due to Bug 1121132.  Can you replace the × in the commit message with 'a multiplication sign'?

Comment 21

[Aaron Raimist [:aaronraimist]]

Attachment: bug1042619_widthxheight_devtools_v4.1.diff

This should be good.

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/0e5635270d32

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0e5635270d32