Closed Bug 1468249 Opened Last year Closed Last year

Toggle buttons are too tall when using touch

Categories

(Firefox :: Menus, defect, P1)

x86
Windows 10
defect

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- verified
firefox63 --- verified

People

(Reporter: mt, Assigned: johannh)

References

Details

(Keywords: regression)

Attachments

(3 files)

Attached image touched.jpg
STR:

Open the hamburger menu by touching it.
The toggle for TP is very tall.

Expected:

It should probably not get taller when using touch, but be centered instead.
Hah, good point. Thanks for filing this.
Blocks: 1462468
Keywords: regression
Priority: -- → P2
Is this the right component? (maybe it doesn't matter)?
Flags: needinfo?(jhofmann)
Probably rather menus, I guess. I can take this.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Component: Tracking Protection → Menus
Flags: needinfo?(jhofmann)
Priority: P2 → P1
Let's mark as fix-optional for 62, but it would be cool to land a patch in 62.
This patch fixes the visual issue, though there's still bug 1468311 which is slightly worse with touch input, IMO.
Comment on attachment 8988299 [details]
Bug 1468249 - Avoid resizing the TP toggle in the main menu in touch mode.

https://reviewboard.mozilla.org/r/253564/#review260324

::: browser/themes/shared/customizableui/panelUI.inc.css:617
(Diff revision 1)
>  }
>  
> +/* Don't resize the toggle in touch mode. */
> +#appMenu-popup[touchmode] #appMenu-tp-toggle {
> +  margin-top: 12px;
> +  margin-bottom: 12px;

Can we add some kind of invisible padding within the button so that it's usable in touch mode without looking ridiculous?
Attachment #8988299 - Flags: review?(dao+bmo)
This is the best I can do, I think. This fixes bug 1468311 in the same commit, the button is still the same size, but at least the user doesn't accidentally close the main menu by tapping outside anymore.
Comment on attachment 8988299 [details]
Bug 1468249 - Avoid resizing the TP toggle in the main menu in touch mode.

https://reviewboard.mozilla.org/r/253564/#review266310
Attachment #8988299 - Flags: review?(dao+bmo) → review+
Blocks: 1468311
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cacad1697a69
Avoid resizing the TP toggle in the main menu in touch mode. r=dao
https://hg.mozilla.org/mozilla-central/rev/cacad1697a69
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
I can confirm this issue is fixed, I tested using Windows Surface Pro, on Fx 63.0a1 (2018-07-26), both x86, x64.
Status: RESOLVED → VERIFIED
Comment on attachment 8988299 [details]
Bug 1468249 - Avoid resizing the TP toggle in the main menu in touch mode.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1462468
[User impact if declined]: Ugly tracking protection toggle when opening the main menu via touch
[Is this code covered by automated tests?]: No, it's CSS-only
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's a very short and straightforward front-end patch that has baked in Nightly for a while now.
[String changes made/needed]: None
Attachment #8988299 - Flags: approval-mozilla-beta?
Comment on attachment 8988299 [details]
Bug 1468249 - Avoid resizing the TP toggle in the main menu in touch mode.

CSS only, regression fix, Beta62+
Attachment #8988299 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This failed to graft for beta. I got:

grafting 490876:cacad1697a69 "Bug 1468249 - Avoid resizing the TP toggle in the main menu in touch mode. r=dao"
merging browser/components/customizableui/content/panelUI.inc.xul
warning: conflicts while merging browser/components/customizableui/content/panelUI.inc.xul! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')

Johann, could you please take a look at this? Thank you.
Flags: needinfo?(jhofmann)
Attached patch Patch for BetaSplinter Review
Updated the patch to work in beta, thanks!
Flags: needinfo?(jhofmann) → needinfo?(csabou)
Flags: needinfo?(csabou)
I can confirm this issue is fixed on Fx 62.0b15. I verified using Windows Surface Pro.
Duplicate of this bug: 1478827
You need to log in before you can comment on or make changes to this bug.