1394361 - (photon) tab title is cropped and not align to close button

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Jing-wei Wu [:jwu]]

Attachment: title cropped and misaligned to close button

In Pixel C, the tab title in tabs tray is cropped and not align to the close button.

Check the attachment for screenshot, I guess this is a side-effect because of the fix of bug 1390670. 

:nechen, would you like to take a look?

Comment 1

[Jan Henning [:JanH]]

(In reply to Jing-wei Wu [:jwu] from comment #0)
> In Pixel C, the tab title in tabs tray is cropped and not align to the close
> button.

I don't see the misalignment on my phone, but the title cropping issue is probably generic and not restricted to just the Pixel C.

Comment 2

[Jan Henning [:JanH]]

Note that in fact we already have some code that uses a TouchDelegate to increase the hit target size of the close button:
https://dxr.mozilla.org/mozilla-central/rev/d10c97627b51a226e19d0fa801201897fe1932f6/mobile/android/base/java/org/mozilla/gecko/tabs/TabsLayoutItemView.java#103

So I'd say the correct course of action would be backing out bug 1390670 again and then verifying that growCloseButtonHitArea() is working as intended (and maybe experimenting with tweaking the target hit area size from 40 dp to 48 dp).

Comment 3

[Jan Henning [:JanH]]

Testing e.g. on Release with the help of the "Pointer location" and "Show layout bounds" developer options, that code does seem to be working as intended - the hit target area of the close button is extended to a rectangle measuring 40 x 40 dp from the outer edges of the TabsLayoutItemView.

Comment 4

[Nevin Chen(Not active on Bugzilla)]

Attachment: Bug 1394361 - Tab title is cropped and not align to close button.

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

Comment 5

[Jan Henning [:JanH]]

(In reply to Jan Henning [:JanH] from comment #2)
> So I'd say the correct course of action would be backing out bug 1390670
> again and then verifying that growCloseButtonHitArea() is working as
> intended (and maybe experimenting with tweaking the target hit area size
> from 40 dp to 48 dp).

Although this has the drawback of not increasing the hit target size for "Touch to explore" a11y mode. Since as far as I've read, "Touch to explore" works through hover events, it might be possible to implement a "HoverDelegate" to redirect hover events as well?

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8902132 [details]
Bug 1394361 - Tab title is cropped and not align to close button.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/173578/diff/1-2/

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8902132 [details]
Bug 1394361 - Tab title is cropped and not align to close button.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/173578/diff/2-3/

Comment 8

[Jing-wei Wu [:jwu]]

Comment on attachment 8902132 [details]
Bug 1394361 - Tab title is cropped and not align to close button.

https://reviewboard.mozilla.org/r/173578/#review179354

Comment 9

[Nevin Chen(Not active on Bugzilla)]

> (In reply to Jan Henning [:JanH] from comment #2)
> Although this has the drawback of not increasing the hit target size for
> "Touch to explore" a11y mode. Since as far as I've read, "Touch to explore"
> works through hover events, it might be possible to implement a
> "HoverDelegate" to redirect hover events as well?

Let's fix the UI lauyout in this bug. And make a follow up bug for a11y issue if any?

Comment 10

[Jan Henning [:JanH]]

The whole UI business is precisely because of a11y in the first place - but sure, go ahead. I'll give my idea a try and report back about its success later.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56a51abed4ff
Tab title is cropped and not align to close button. r=jwu

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/56a51abed4ff

Comment 13

[Oana Horvath [:ohorvath]]

Verified as fixed in Nightly 57 (2017-08-31).
Device: HTC Nexus 9 (Android 7.1.1)

Comment 14

[Jan Henning [:JanH]]

(In reply to Oana Horvath from comment #13)
> Verified

For certain values of "Verified" - the tab title is still cropped.