Closed
Bug 1320605
Opened 8 years ago
Closed 8 years ago
Fade to white for border around URL box stuck for long URLs
Categories
(Firefox for Android Graveyard :: Awesomescreen, defect)
Tracking
(fennec51+, firefox50 wontfix, firefox51 wontfix, firefox52 verified, firefox53 verified)
VERIFIED
FIXED
Firefox 53
People
(Reporter: arvinder.bhathal, Assigned: cnevinchen)
References
Details
Attachments
(2 files)
810.99 KB,
image/png
|
Details | |
58 bytes,
text/x-review-board-request
|
sebastian
:
review+
jcristau
:
approval-mozilla-aurora+
jcristau
:
approval-mozilla-beta-
|
Details |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/53.0.2785.34 Safari/537.36 Build ID: 20161114144856 Steps to reproduce: [Nexus 5X, Android 7.0, Build NRD91N, Firefox Nightly v.53.0a1] Visit any website where the URL is longer than the space available inside the URL box. Actual results: The border around the URL box transitions to white along with the text, seemingly to indicate the URL is longer than the space available. However, the right-side of the border is not subject to the transition effect and the resulting overall transition looks unnatural. If Firefox offers a reading view for the website, the transition ends just before the reading view icon. I am not entirely sure this is a bug as the resulting effect may have been intended. The attached image shows a website with Firefox offering a reading view icon. Expected results: The text of the URL should transition to white to indicate a long URL but the border should remain one colour.
Comment 1•8 years ago
|
||
I can't see this on a Pixel XL. @Max: random guess, could this be a regression from the RTL patches?
tracking-fennec: --- → ?
Component: Graphics, Panning and Zooming → Awesomescreen
Flags: needinfo?(max)
Comment 2•8 years ago
|
||
Not likely to be related to RTL patches, RTL only update the left/right to start/end. Also, RTL patch landed on 11/24. And the reported version is build on 11/14.
Flags: needinfo?(max)
Comment 3•8 years ago
|
||
STR: Change overall system font size will reproduce the symptom.
Comment 4•8 years ago
|
||
(In reply to Max Liu [:maliu] from comment #3) > STR: > Change overall system font size will reproduce the symptom. Ah, thanks! Now I can see it. And it is reproducible in all release channels.
status-firefox50:
--- → affected
status-firefox51:
--- → affected
status-firefox52:
--- → affected
status-firefox53:
--- → affected
Reporter | ||
Comment 5•8 years ago
|
||
Ah yes, system font size set to "Large".
Updated•8 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → cnevinchen
Updated•8 years ago
|
tracking-fennec: ? → 51+
Comment hidden (mozreview-request) |
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8816354 [details] Bug 1320605 - The shrunk height just nee to cover the text itself. https://reviewboard.mozilla.org/r/97116/#review97826 ::: mobile/android/base/java/org/mozilla/gecko/widget/FadedMultiColorTextView.java:60 (Diff revision 1) > - final float top = center - getTextSize() + 2; > - final float bottom = center + getTextSize() - 2; > + // The shrunk size just nee to cover the text itself. > + final float top = center - getTextSize()/2 ; > + final float bottom = center + getTextSize()/2 ; The patch looks good, but the formatting is off. It should look like this: > getTextSize() / 2;
Attachment #8816354 -
Flags: review?(s.kaspari) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 11•8 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/594afa97ff78 The shrunk height just nee to cover the text itself. r=sebastian
Keywords: checkin-needed
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/594afa97ff78
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Updated•8 years ago
|
Flags: needinfo?(s.kaspari)
Comment 13•7 years ago
|
||
Sebastian, you marked it as tracking for 51. It might be too late for 51 but is it something you would like to uplift to 52? Thanks
Flags: needinfo?(s.kaspari)
Comment 14•7 years ago
|
||
Yes, let's uplift this. Nevin, can you request it?
Flags: needinfo?(s.kaspari) → needinfo?(cnevinchen)
Assignee | ||
Comment 15•7 years ago
|
||
Comment on attachment 8816354 [details] Bug 1320605 - The shrunk height just nee to cover the text itself. Approval Request Comment [Feature/regressing bug #]: The shrunk height just need to cover the text itself. [User impact if declined]: User sees white gradient cause the height is larger than the input box height. [Describe test coverage new/current, TreeHerder]: manually check the gradient color and height [Risks and why]: Low. It just changes gradient color and its height [String/UUID change made/needed]: -
Flags: needinfo?(cnevinchen)
Attachment #8816354 -
Flags: approval-mozilla-aurora?
Comment 16•7 years ago
|
||
Comment on attachment 8816354 [details] Bug 1320605 - The shrunk height just nee to cover the text itself. fix fennec url box gradient for aurora52 FWIW both in the commit message and code comment, you wrote "nee" for "needs".
Attachment #8816354 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 17•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/6180973a5211
Comment 18•7 years ago
|
||
Verified as fixed on Aurora 52.0a2 (2017-01-13) and Nightly 53 (2017-01-13), with the following devices: -LG G4 (Android 5.1) phone; -HTC Nexus 9 (Android 7.0) tablet; -Asus Transformer Pad (Android 4.2.1) tablet.
Comment 20•7 years ago
|
||
Comment on attachment 8816354 [details] Bug 1320605 - The shrunk height just nee to cover the text itself. Approval Request Comment [Feature/regressing bug #]: The shrunk height just need to cover the text itself. [User impact if declined]: User sees white gradient cause the height is larger than the input box height. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: In bug [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: Very low. [Why is the change risky/not risky?]: It just changes gradient color and its height [String changes made/needed]: None I realize this is a long shot at this point, but I thought no harm in trying. This was a partner reported bug, and they are shipping 51 so I would love to see this in 51 if we can. It's a two line change.
Attachment #8816354 -
Flags: approval-mozilla-beta?
Comment 21•7 years ago
|
||
I'm confused by this beta approval request. beta is 52 which had this patch uplifted already in comment 17. 51 is release, and 2 weeks before the 52 release it's fairly unlikely there's be another 51 build.
Flags: needinfo?(mozilla)
Comment 22•7 years ago
|
||
I guess I didn't realize how close we were to 51. If it's a no go for 51, that's OK. just thought I would try.
Flags: needinfo?(mozilla)
Updated•7 years ago
|
Comment 23•7 years ago
|
||
Comment on attachment 8816354 [details] Bug 1320605 - The shrunk height just nee to cover the text itself. This is already in beta...
Attachment #8816354 -
Flags: approval-mozilla-beta? → approval-mozilla-beta-
Comment 24•7 years ago
|
||
Sorry, got my releases confused.
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•