Closed Bug 850814 Opened 8 years ago Closed 8 years ago

Menu's curve should not overlap tab curve

Categories

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

ARM
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 22
Tracking Status
firefox20 --- verified
firefox21 --- verified

People

(Reporter: sriram, Assigned: sriram)

References

Details

Attachments

(3 files)

Changing the highlights in Bug 848707 revealed that the menu overlaps the tabs button's curves. This should be avoided.
Attached patch PatchSplinter Review
This aligns the curves.
Attachment #724561 - Flags: review?(mark.finkle)
Blocks: 848707
Attachment #724561 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/f345667b192d
Assignee: nobody → sriram
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Comment on attachment 724561 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 831021
User impact if declined: A small line a black pixels due to mis-alignment.
Testing completed (on m-c, etc.): Landed in m-c on 03/14
Risk to taking this patch (and alternatives if risky): None. Just pixel alignment. (From 17dp, I made it 19dp, instead of 18dp).
String or UUID changes made by this patch: None.
Attachment #724561 - Flags: approval-mozilla-beta?
Attachment #724561 - Flags: approval-mozilla-aurora?
Comment on attachment 724561 [details] [diff] [review]
Patch

Fixes low risk alignment glitch.
Attachment #724561 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
To be considered for Beta at this point (the final two weeks) we'd need to see a screenshot. I wasn't able to find this pixel mis-alignment through normal inspection. Chances are we wontfix for FF20. Regardless of risk, we should only take change we need in the final two weeks.
(In reply to Alex Keybl [:akeybl] from comment #6)
> To be considered for Beta at this point (the final two weeks) we'd need to
> see a screenshot. I wasn't able to find this pixel mis-alignment through
> normal inspection. Chances are we wontfix for FF20. Regardless of risk, we
> should only take change we need in the final two weeks.

This is seen on hdpi phones like Motorola Droid Razr. This is not seen on xhdpi phones.
Attached image Before/after screenshot
This attachment shows the before (3/14 build) and after (3/15 build) on the private browsing start page. The key thing to notice is that the sliver of grey on the tab shape is much less in the new version.

One thing I did notice, and I have a screenshot of it too, is that the page would start scroll down some, like it was being dragged down even when I wasn't touching the screen. I'm not sure if this is a regression related to this bug or not, I'll upload a screenshot of it next anyways.
(In reply to Jared Wein [:jaws] from comment #8)
> Created attachment 725615 [details]
> Before/after screenshot
> 
> This attachment shows the before (3/14 build) and after (3/15 build) on the
> private browsing start page. The key thing to notice is that the sliver of
> grey on the tab shape is much less in the new version.

Thanks.

> One thing I did notice, and I have a screenshot of it too, is that the page
> would start scroll down some, like it was being dragged down even when I
> wasn't touching the screen. I'm not sure if this is a regression related to
> this bug or not, I'll upload a screenshot of it next anyways.

That's a regression from dynamic toolbar only on Nightly -- unrelated to this patch.

Alex, could you approve this patch?
As described in the previous comment, I noticed this when I upgraded but I'm not sure if it is related to this bug. After touching the page it went away and wouldn't come back.
Verified with HTC Sensation 4G running Android 4.0.3.
Status: RESOLVED → VERIFIED
Comment on attachment 724561 [details] [diff] [review]
Patch

If this was caused by bug 848707 - that didn't even land to FF20 so why does this need to be uplifted to that branch?
Attachment #724561 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment on attachment 724561 [details] [diff] [review]
Patch

Sorry, I see now that it's Bug 831021 causing the issue - will take this low risk fix now for Beta so it can be in tomorrow's build.  Please uplift asap.
Attachment #724561 - Flags: approval-mozilla-beta- → approval-mozilla-beta+
Verified fixed on:
-build: Firefox for Android 20 Beta 6 (2013-03-21)
-device: Samsung Galaxy Nexus
-OS: Android 4.1.1
Verified on Firefox for Android 21.0b7 using: Nexus 4 (4.2.2)
You need to log in before you can comment on or make changes to this bug.