596151 - when in touch-screen mode app tabs aren't as tall as real tabs, therefore self-conscious

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Mike Shaver (:shaver emeritus)]

I am on a laptop with a touch screen, and that means I get extra big tabs for use with my dialing wand.  My app tabs are late bloomers.

http://grab.by/grabs/ff70ee70495b94dad9714876f1b0762c.png

Comment 1

[:Felipe Gomes (needinfo for replies!)]

Thanks for filing Shaver, I have a WIP patch but didn't file a bug about it yet.

The problem is that the min-height touch-enabled style is in the .tabbrowser-tabs element, but as the app tabs are position:fixed they don't stretch to the whole height of the container.
Also, the tabs are taller than they should be due to some recent theme changes.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch v1

Some similar reasons here for using -moz-calc as there was in bug 579683 (need to account for the height represented by the padding and borders, currently 7px)

The 2px adjustment are due to some recent unwanted increases in that height, probably fallout from various theme tweaking changes (e.g. moving the border from the TabsToolbar to the navigator-toolbox).
With the -2px in the calc the dimensions go back as implemented in bug 572472

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 479702 [details] [diff] [review]
Patch v1

> .tabbrowser-tabs:-moz-system-metric(touch-enabled) {
>-  min-height: 7.3mozmm;
>+  min-height: -moz-calc(7.3mozmm - 2px);
> }

I guess one extra pixel is due to DirectWrite -- we need to fix that globally, not just for touch-enabled.

(In reply to comment #1)
> The problem is that the min-height touch-enabled style is in the
> .tabbrowser-tabs element, but as the app tabs are position:fixed they don't
> stretch to the whole height of the container.

So how about always setting the min-height on the tab elements, e.g. .tabbrowser-tab or .tab-content?

Comment 4

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch v2

(In reply to comment #3)
> So how about always setting the min-height on the tab elements, e.g.
> .tabbrowser-tab or .tab-content?

The min-height on .tabbrowser-tab produced odd results like a 1px height difference between a single tab and two or more tabs, but .tab-content worked perfectly.

 The size calculations is the same as the previous patch. I've  verified that it results in the same height with DirectWrite on and off.


Also moved the declaration from the "Tab-strip" section to the "Tab" section inside the file

Comment 5

[:Felipe Gomes (needinfo for replies!)]

(I thought this was already marked as a blocker)

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 486956 [details] [diff] [review]
Patch v2

>+.tab-content:-moz-system-metric(touch-enabled) {
>+  min-height: -moz-calc(7.3mozmm - 2px - 7px); /* subtract existing borders and paddings from the height. */
>+}

What padding?

Comment 7

[:Felipe Gomes (needinfo for replies!)]

I added "borders and paddings" to the comment more as a generic annotation (i.e. if some changes in borders or paddings happen in the future this might need to be adjusted).

I don't know if atm. all is just borders or if there are any paddings at play here (I couldn't find any in DOMi), but I believe that 1px of the fine tuning here was needed due to the margin/padding change from bug 590873.

Comment 8

[Dão Gottwald [:dao]]

DOMi reports a 4px top and 3px bottom border -- what are the other two pixels? If we don't actually want tabs to be 7.3 mm tall, we shouldn't subtract some pixels but use a lower mm value.

Comment 9

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch v3

Ok yeah, I incorporated the -2px in the mm value now. I was keeping the 7.3mm to keep the original measurements but it's better to have the final value wanted there.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 491358 [details] [diff] [review]
Patch v3

>+.tab-content:-moz-system-metric(touch-enabled) {
>+  min-height: -moz-calc(6.8mozmm - 7px); /* subtract existing borders from the height. */
>+}

nit: "subtract borders from the desired height"

There's a @media all and (-moz-touch-enabled) section some lines below. Can you move this there and drop :-moz-system-metric(touch-enabled)?

Comment 11

[:Felipe Gomes (needinfo for replies!)]

Attachment: Patch to land

Comment 12

[:Felipe Gomes (needinfo for replies!)]

http://hg.mozilla.org/mozilla-central/rev/1709bbd7964f