Nexus 7 2013, current m-c. If the URL of the visited page is longer than what fits in the Awesomebar, the domain highlighting (marking the hostname darker/bolder than the rest of the URL) stops working. This makes phishing easier.

It'd be cool if we could land this for new tablet (soft dependency).

I disabled the fading parts of FadingTextView (by commenting out [1]) and the issue appears to be solved. Looking for a solution... [1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/FadedTextView.java?rev=2878bf6cbc1f#79

Regression window-wanted: mozilla-central: good build: 24-09-2014 bad build: 25-09-2014 pushlog:http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1e2993c99323&tochange=1735ff2bb23e

That bisection points at bug 1061508 but that was already somewhat obvious from comment 3 as well.

The host coloring appears to happen here: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/ToolbarDisplayLayout.java#372 updateGradientShader and FadedTextGradient seem to assume the text only has one color: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/FadedTextView.java#62 https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/FadedTextView.java#89 This code dates back to: http://sriramramani.wordpress.com/2013/06/06/ellip-sis/ note the remark at the end: "Update: Thanks to Romain Guy! This can be achieved by using android:fadingEdgeLength and android:requiresFadingEdge="horizontal" (in ICS). On pre-ICS phones, it’s enabled using android:fadingEdge. Sigh! So much work for something existing! :("

Thanks for the deep dive, GCP.

To test on a pre-ICS device.

I presume for the Awesomebar the bad performance effects are much less pronounced? http://sriramramani.wordpress.com/2013/06/06/ellip-sis/#comment-522

Comment on attachment 8536828 [details] [diff] [review] Part 1: Change BrowserToolbar to use Android's fading edge Review of attachment 8536828 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/resources/layout/toolbar_display_layout.xml @@ +32,5 @@ > android:layout_weight="1.0" > + gecko:autoUpdateTheme="false" > + android:ellipsize="none" > + android:singleLine="true" > + android:fadingEdge="horizontal" Align all other attributes in this tag with the android:id attribute

While still maintaining 60 FPS on my N7, this seems like a pretty unacceptable drop in performance for such a inconsequential feature. Looking into a solution where we draw over the text with a rectangle that uses a linear gradient filter.

(In reply to Michael Comella (:mcomella) from comment #13) > Created attachment 8537546 [details] > Post-patch: perf graph > > While still maintaining 60 FPS on my N7, this seems like a pretty > unacceptable drop in performance for such a inconsequential feature. Agreed. Good call.

To give you an idea where I'm going with this.

Note that this does not work for RTL.

To show how large the drawn rectangle is compared to font size.

Discrepencies in perf from comment 19 to comment 12 could also be the number of tabs open, the displayed page content, and (not likely) the text color.

Comment on attachment 8543025 [details] [diff] [review] Part 2: Add FadedMultiColorTextView Review of attachment 8543025 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/widget/FadedMultiColorTextView.java @@ +68,5 @@ > + final boolean needsNewGradient = (backgroundGradient == null || > + backgroundGradient.getBackgroundColor() != backgroundColor || > + backgroundGradient.getEndRight() != gradientEndRight); > + > + if (needsEllipsis() && needsNewGradient) { You already check needsEllipsis() in onDraw(), so drop this to prevent the extra layout calculations.

hg qadd. hg qref. -_-

(In reply to Brian Nicholson (:bnicholson) from comment #21) > You already check needsEllipsis() in onDraw(), so drop this to prevent the > extra layout calculations. Good call. I chose to pass it in as an arg instead though - keeping the needsEllipsis check inside updateGradientShader is more future-proof.

https://hg.mozilla.org/integration/fx-team/rev/7cbfc2f0ee86 https://hg.mozilla.org/integration/fx-team/rev/2a4ba57bf69f

Comment on attachment 8542906 [details] [diff] [review] Part 1: Make FadedTextView abstract and move current implementation to FadedSingleColorTextView Note: the reviewer is incorrect on the uploaded version of the patch - please change to bnicholson. This request is for parts 1 & 2. Approval Request Comment [Feature/regressing bug #]: Domain highlighting has always had this issue, but it is much more noticeable on new tablet (FF 36) since the url is shown by default. [User impact if declined]: Users with long urls will not have the text highlighted - makes tablet look less polished. [Describe test coverage new/current, TBPL]: None [Risks and why]: Worst case, we break all of our TextViews that shorten the text with an ellipsis-technique (we use fading) because part 1 makes the implementing View an abstract super-class and sub-class. More likely, the alternative faded text implementation (part 2; only used in toolbar display text) is underperformant, does not work in certain edge cases (e.g. private browsing), or completely messes with the toolbar text. It closes follows the code paradigm of part 1, however, so I doubt this will be an issue. [String/UUID change made/needed]: N/A

https://hg.mozilla.org/mozilla-central/rev/7cbfc2f0ee86 https://hg.mozilla.org/mozilla-central/rev/2a4ba57bf69f

Verified as fixed in Firefox for Android 37.0a1 (2015-01-05) Device: Nexus 7 (Android 5.0.1)

https://hg.mozilla.org/releases/mozilla-aurora/rev/b14cfa132801 https://hg.mozilla.org/releases/mozilla-aurora/rev/f96aef87f401

Verified as fixed in Firefox for Android 36.0a2 (2015-01-06) Device: Asus Transformer Pad TF300T (Android 4.2.1)

I will mark this as Verified fixed based on comment 27 and comment 28.

It’s not fixed in 51 I hope you can check this issue. Thanks and BR.

lgbrowser5@gmail.com: If you are experiencing the exact same problem, please open a new bug. Or is what you are seeing bug 1163963?