Closed Bug 1320605 Opened 3 years ago Closed 3 years ago

Fade to white for border around URL box stuck for long URLs

Categories

(Firefox for Android :: Awesomescreen, defect)

53 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 53
Tracking Status
fennec 51+ ---
firefox50 --- wontfix
firefox51 --- wontfix
firefox52 --- verified
firefox53 --- verified

People

(Reporter: arvinder.bhathal, Assigned: cnevinchen)

References

Details

Attachments

(2 files)

Attached image Screenshot_2.png
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.
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)
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)
STR:
Change overall system font size will reproduce the symptom.
(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.
Ah yes, system font size set to "Large".
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → cnevinchen
tracking-fennec: ? → 51+
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+
Fixed in the 3rd patch. Thanks!
Flags: needinfo?(s.kaspari)
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
https://hg.mozilla.org/mozilla-central/rev/594afa97ff78
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Flags: needinfo?(s.kaspari)
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)
Yes, let's uplift this. Nevin, can you request it?
Flags: needinfo?(s.kaspari) → needinfo?(cnevinchen)
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 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+
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.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 1163963
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?
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)
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)
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-
Sorry, got my releases confused.
You need to log in before you can comment on or make changes to this bug.