1348267 - Grid Inspector: line numbers on retina display are too small

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Matteo Ferretti [:zer0] [:matteo]]

This is a visual regression from bug 1303158: now that we don't scale the canvas' context anymore, but we manually handle the pixel ratio, the size of the font is consistent across display density and zoom.

This is a nice feature – font are not getting bigger or smaller if the user zoom in or out, but they are kept as is – but we have to take in account the display density pixel ratio, otherwise higher is the density, smaller is the font.

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: Bug 1348267 - used display density pixel ratio to scale the font size;

Added also a `getDisplayPixelRatio` method, since we're probably going to use it
more often, instead of doing this math all the time in the code, it will be more
clear.

Review commit: https://reviewboard.mozilla.org/r/121404/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121404/

Comment 2

[Patrick Brosset <:pbro>]

Comment on attachment 8848476 [details]
Bug 1348267 - used display density pixel ratio to scale the font size;

https://reviewboard.mozilla.org/r/121404/#review123432

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eafe3339e243
used display density pixel ratio to scale the font size; r=pbro

Comment 4

[Jen Simmons [:jensimmons]]

> the size of the font is consistent across display density and zoom

I've noticed this as well. Since the numbers are far too tiny at the moment, I want to zoom the page so I can see them, and they do no grow any bigger as the page is zoomed. (Especially when presenting onstage in a live demo.)

They should grow when the user zooms.

Comment 5

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Jen Simmons [:jensimmons] from comment #4)

> > the size of the font is consistent across display density and zoom
> 
> I've noticed this as well. Since the numbers are far too tiny at the moment,
> I want to zoom the page so I can see them, and they do no grow any bigger as
> the page is zoomed. (Especially when presenting onstage in a live demo.)
> 
> They should grow when the user zooms.

They're part of the HUD so they shouldn't zoom IMVHO. You can think about that like sketch or photoshop: you can zoom the content, but the UI is not zooming. The same for the Grid Inspector's line: they shouldn't grown if we zoom.

For example, see the box model's inspector, and the infobar: we don't increase anything when we zoom – we actually want to keep the same ratio.

Said that, I totally understand the use case. We could add a pref to allow the line number to zoom along with the content, however I don't think that would be the best solution, especially for the use case you mentioned; since the grid's lines won't zoom – and they shouldn't, as we don't increase the inspector's lines as said.

With just the numbers bigger, but not the actual grid, the audience in a presentation won't be able to see much.

Therefore my suggestion is, for the specific scenario, to zoom using the OS capabilities (changing resolution, dpi, zoom factor, etc.). That would increase everything, also the grid's lines; so for a presentation it's better.

Comment 6

[Jen Simmons [:jensimmons]]

I agree that in general, for the defaults, the lines should be thicker and the numbers should be bigger. That will help.

I do not think that these numbers are like anything else we have already. We have dev tools, and they zoom. We have browser chrome, and as you point out, like all other software interfaces, they do not zoom. We do not have any devtools that overlay onto the webpage, yet. Although I expect this will be the first of many. It's an incredibly helpful way to interact with CSS. 

You are arguing this new interface should work like the browser itself. I'm arguing it should work like the devtools. And the website within the browser's viewport. Here, let me show you: https://vimeo.com/208904870/611f7b9908

I mentioned a personal usecase above, to make what I was describing more understandable, but I'm not advocating for this for me personally. 

Allowing a developer to zoom the numbers along with their whole page is an important accessibility feature. Why build a new tool that's not a11y?

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

I know this is technically resolved, but we should not just let Jen Simmons' feedback go into the void because the bug is resolved. zer0, can you file a follow based on comment 6.

Comment 9

[Gabriel Luong [:gl] (ΦωΦ)]

I meant follow up

Comment 10

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Jen Simmons [:jensimmons] from comment #6)

> I do not think that these numbers are like anything else we have already. We
> have dev tools, and they zoom.

They zoom independently by the content: they do not zoom when the content zoom.

> We have browser chrome, and as you point out,
> like all other software interfaces, they do not zoom. We do not have any
> devtools that overlay onto the webpage, yet.

That's not accurate. We have plenty of highlighters as the CSS Grid. For me already the Box Model and the infobar is an example of that; but also the Geometry Editor for example, and so on. And we handle them specifically in a way that we do not want to zoom the HUD (also, the labels) when the content is zoomed. That not something the developer is expecting in regular web development - it basically considered as a bug.

> You are arguing this new interface should work like the browser itself. I'm
> arguing it should work like the devtools. And the website within the
> browser's viewport. Here, let me show you:

As mentioned above, the devtools doesn't zoom when the page is zoomed. That's what I'm arguing, that the HUD shouldn't reflect the zooming factor of the page. As designer I zoom a lot in my content (on browser, on Photoshop, on Sketch) but I don't want that my interface is zooming at the same time.

For instance, I would find it annoying and not useful if zooming the page will automatically zooming the devtools as well, in Firefox.

> I mentioned a personal usecase above, to make what I was describing more
> understandable, but I'm not advocating for this for me personally. 

I totally understand, and I totally understand your use case. In fact, I think that this use case is valid; but I don't think that this is the way it should be addressed. As said, the best approach would be using the OS capabilities, or modifying the value of `layout.css.devPixelsPerPx` preference. 
 
> Allowing a developer to zoom the numbers along with their whole page is an
> important accessibility feature.

That's what we're not agreed I think. I don't think the two things should be connected. If you think we should have a way to zoom our overlays (or at least the text content of it) for accessibility reason, I can understand and can be discussed. But, as for the devtools, it shouldn't be done along with the page zoom. It should be independent. And, if we want to do that, it should be done for all our overlays, not just one: we should be consistent.

So, specifically for the Grid Inspector (highlighter), the bug 1341612 should mitigate the fact that the number are too small currently.
If we want have an option, in addition to the ones I already mentioned, to zoom the highlighters' (text) content for accessibility reason, I think should be discussed in a more follow up bug.

Comment 11

[Matteo Ferretti [:zer0] [:matteo]]

Filed bug 1348643. Not sure if it was worth to copy all the comments, but at least we have a bug for that.