Closed Bug 1059450 Opened 6 years ago Closed 6 years ago

Regression: Thin pixel bar present next to the back button on tablets in the new toolbar

Categories

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

34 Branch
ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 34
Tracking Status
firefox34 --- affected
fennec 34+ ---

People

(Reporter: aaronmt, Assigned: lucasr)

References

Details

(Keywords: regression, reproducible)

Attachments

(3 files)

See screenshot.

--
Sony Xperia Z (Nightly 08/27)
Attached image screenshot.png
On long-tap, the bar extends beyond the border of the address-bar
Assignee: nobody → lucasr.at.mozilla
(In reply to Aaron Train [:aaronmt] from comment #1)
> Created attachment 8480051 [details]
> screenshot.png
> 
> On long-tap, the bar extends beyond the border of the address-bar

Is this related to this bug?
Comment on attachment 8480530 [details] [diff] [review]
Fix shape dimention in BackButton (r=mcomella)

This was a thinko of mine. The button shape path should fill the whole view bounds and the border stroke is painted half in & half out of the path.
Attachment #8480530 - Flags: review?(michael.l.comella)
tracking-fennec: ? → 34+
Comment on attachment 8480530 [details] [diff] [review]
Fix shape dimention in BackButton (r=mcomella)

Review of attachment 8480530 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not sure how this fixes the issue - can you elaborate on why this draws a line next to the back button?
Attachment #8480530 - Flags: review?(michael.l.comella) → feedback-
Comment on attachment 8480530 [details] [diff] [review]
Fix shape dimention in BackButton (r=mcomella)

(In reply to Michael Comella (:mcomella) from comment #5)
> Comment on attachment 8480530 [details] [diff] [review]
> Fix shape dimention in BackButton (r=mcomella)
> 
> Review of attachment 8480530 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not sure how this fixes the issue - can you elaborate on why this draws
> a line next to the back button?

Apologies, I should have given more context. The line next to the back button is an artifact caused by the difference between the view and shape bounds. If the shape is smaller than the view, the saveLayer() call will capture stuff that is 'outside' the shape. See:
http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/CanvasDelegate.java#41

Another way to fix this could be to use the shape dimensions here:
http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/BackButton.java#60

But I find it cleaner to simply have the shape matching the view bounds.
Attachment #8480530 - Flags: feedback- → review?(michael.l.comella)
Comment on attachment 8480530 [details] [diff] [review]
Fix shape dimention in BackButton (r=mcomella)

Review of attachment 8480530 [details] [diff] [review]:
-----------------------------------------------------------------

r+, to avoid the lengthy review back-and-forth cycle, but I still have questions.

If we're drawing a circle to the view bounds, how does this eliminate a line from the View where the circle is not drawn (e.g. top-right corner, outside the boundaries of the circle)? Does the circle draw transparency over the smallest rect containing the circle where the circle is not visible?
Attachment #8480530 - Flags: review?(michael.l.comella) → review+
(In reply to Michael Comella (:mcomella) from comment #7)
> Comment on attachment 8480530 [details] [diff] [review]
> Fix shape dimention in BackButton (r=mcomella)
> 
> Review of attachment 8480530 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+, to avoid the lengthy review back-and-forth cycle, but I still have
> questions.
> 
> If we're drawing a circle to the view bounds, how does this eliminate a line
> from the View where the circle is not drawn (e.g. top-right corner, outside
> the boundaries of the circle)? Does the circle draw transparency over the
> smallest rect containing the circle where the circle is not visible?

Pretty much. The line is simply a region of the view that is not 'reached' by the alpha mask defined in ShapedButton's mPath.
https://hg.mozilla.org/mozilla-central/rev/dbedb33e649d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
You need to log in before you can comment on or make changes to this bug.