Closed Bug 1331431 Opened 3 years ago Closed 3 years ago

Faded Text View on the URL bar should also fade the bottom part of letters such as 'g', 'y' etc

Categories

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

All
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
fennec 52+ ---
firefox50 --- unaffected
firefox51 --- unaffected
firefox52 --- verified
firefox53 --- fixed

People

(Reporter: itiel_yn8, Assigned: cnevinchen)

Details

(Keywords: regression)

Attachments

(2 files)

Attached image Screenshot
STR:
1. Open Nightly
2. Open a website whose URL has letters such as 'y', 'g' etc where the text would get faded out. For the sake of this STR, open a fake URL, something like www.yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy.com
3. Observe the faded out text

Actual results:
The bottom part of the letter 'y' isn't being faded out.

Expected results:
It should.

See screenshot attached.
This is a regression.
tracking-fennec: --- → ?
Keywords: regression
OS: Unspecified → Android
Hardware: Unspecified → All
Could be caused by bug 1320605?
Flags: needinfo?(cnevinchen)
tracking-fennec: ? → +
Priority: -- → P2
Maybe this should track 52. It's a regression that started in 52 and we should avoid letting it ride to release.
Flags: needinfo?(whuang)
Assignee: nobody → cnevinchen
Status: NEW → ASSIGNED
Comment on attachment 8827838 [details]
Bug 1331431 - Use baseline and FontMetrics instead of textview's center and text size to get the real text height.

https://reviewboard.mozilla.org/r/105424/#review106242

This patch contains a patch file. :)
Attachment #8827838 - Flags: review?(s.kaspari) → review-
(In reply to Sebastian Kaspari (:sebastian) from comment #4)
> Maybe this should track 52. It's a regression that started in 52 and we
> should avoid letting it ride to release.

agree+1
tracking-fennec: + → 52+
Flags: needinfo?(whuang)
Priority: P2 → P1
Textview doesn't who text in the center but on baseline. I've made a new patch.
Flags: needinfo?(cnevinchen)
Comment on attachment 8827838 [details]
Bug 1331431 - Use baseline and FontMetrics instead of textview's center and text size to get the real text height.

https://reviewboard.mozilla.org/r/105424/#review106644

::: mobile/android/base/java/org/mozilla/gecko/widget/FadedMultiColorTextView.java:20
(Diff revision 3)
>  import android.graphics.LinearGradient;
>  import android.graphics.Paint;
> +import android.graphics.Rect;
>  import android.graphics.Shader;
>  import android.util.AttributeSet;
> +import android.util.Log;

nit: unnecessary import - from debugging? :)
Attachment #8827838 - Flags: review?(s.kaspari) → review+
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/21e0b0c5284a
Use baseline and FontMetrics instead of textview's center and text size to get the real text height. r=sebastian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/21e0b0c5284a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Please request Aurora approval on this when you get a chance.
Flags: needinfo?(cnevinchen)
I can confirm this is fixed on latest Nightly.
Comment on attachment 8827838 [details]
Bug 1331431 - Use baseline and FontMetrics instead of textview's center and text size to get the real text height.

Approval Request Comment
[Feature/Bug causing the regression]:Faded Text View on the URL bar should cover letters with descending ('g', 'y')
[User impact if declined]: User will see broken UI
[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]: no
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: Just fixing UI
[String changes made/needed]: no
Flags: needinfo?(cnevinchen)
Attachment #8827838 - Flags: approval-mozilla-aurora?
Comment on attachment 8827838 [details]
Bug 1331431 - Use baseline and FontMetrics instead of textview's center and text size to get the real text height.

fix fennec regression in beta52
Attachment #8827838 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Verified as fixed in build 52.0b1 (52 Beta 1);
Device: Huawei MediaPad M2 (Android 5.1.1).
You need to log in before you can comment on or make changes to this bug.