Closed Bug 1524857 Opened 10 months ago Closed 10 months ago

URL alignment in location bar

Categories

(Firefox for Android :: Awesomescreen, defect, P1)

Firefox 65
All
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 67
Tracking Status
firefox65 --- wontfix
firefox66 + verified
firefox67 --- verified

People

(Reporter: keul, Assigned: JanH)

Details

Attachments

(4 files)

The location bar does not display the TLD first.

On Firefox for desktop, the domain is displayed in black and the rest in grey. Because domain name is more important than any other part of the URL.

On Firefox mobile, the domain name is partially masked while it could have been displayed in full before displaying subdomain or the rest of the URL.

Note than if the domain is not fully displayed, we should be certain that the fact that it's truncated is displayed.

What should be done:
1° display the URL from left to right
2° If URL overflow, do the overflow on the left, the .TLD/ should always be visible on the right
3° if the subdomains overflow, display them while keeping the TLD visible, try to display the rest of the URL too if there things after the /

What you suggest is in fact what should normally happen, but it seems that currently we don't run domain highlighting when the page load ends up in an error page, and the URL justification code depends on the domain highlighting in order to right-justify the TLD.

Desktop apparently always does the eTLD+1 highlighting, so I guess that's what should actually happen on Android as well.

Status: UNCONFIRMED → NEW
Component: Theme and Visual Design → Awesomescreen
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → All
Assignee: nobody → jh+bugzilla

Currently, the Android front-end uses a tab's base domain both for permission
doorhangers, as well as for doing domain highlighting in the URL bar. The base
domain in turn is based on the document's nodePrincipal's URI.

As per bug 1325955, the nodePrincipal is the right choice for permission
prompts, but it causes some problems for domain highlighting instead: For error
pages for example, the nodePrincipal's URI will be some variety of
about:neterror, which means that the front-end won't be able to do any domain
highlighting based on that, since
a) we don't generate any baseDomain anyway because the URI's scheme isn't
HTTP(S)/FTP, and
b) even if we did, the nodePrincipal's baseDomain won't match the contents of
the URI displayed in the URL bar.

Therefore, we want to separate these two concerns, and generate two baseDomains:
One based on the nodePrincipal for use in permission doorhangers and the like,
and one based on the display URI, which going forward will power our domain
highlighting.

That way, domain highlighting (and therefore the URL justification code that
right-justifies the TLD within the URL bar) can run even on error pages.

This also means that the workaround from bug 1479311 for blocking javascript:
URIs from being highlighted in ToolbarDisplayLayout is no longer required -
the base domain for domain highlighting is now being generated from the same
URI that actually ends up being displayed in the URL bar, and as such the
existing checks in browser.js for only generating a base domain for HTTP(S)/
FTP-URIs, but not any other schemes, finally work the way they are intended.

Jan, once this lands, we may want to uplift to beta, if you think it will not be too risky.

Flags: needinfo?(jh+bugzilla)

(In reply to Liz Henry (:lizzard) (use needinfo) from comment #5)

Jan, once this lands, we may want to uplift to beta, if you think it will not be too risky.

Should be possible.

Pushed by mozilla@buttercookie.de:
https://hg.mozilla.org/integration/autoland/rev/858b4a542bf4
Part 0: Use short form where possible for defining properties. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/d32c9ce0d366
Part 1: Separate base domain for doorhangers from base domain used for domain highlighting. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/f110ae482730
Part 2: Use display URI's base domain for domain highlighting. r=snorp
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Comment on attachment 9041241 [details]
Bug 1524857 - Part 2: Use display URI's base domain for domain highlighting. r?snorp

Beta/Release Uplift Approval Request

Feature/Bug causing the regression

URL bar

User impact if declined

Domain highlighting and right-aligning of the TLD won't work when the load ends up in an error page

Is this code covered by automated tests?

No

Has the fix been verified in Nightly?

Yes

Needs manual test from QE?

No

If yes, steps to reproduce

List of other uplifts needed

None

Risk to taking this patch

Low

Why is the change risky/not risky? (and alternatives if risky)

Existing code for generating a base domain has simply been extracted into a helper function, so we can now generate a base domain for URL bar domain highlighting that is based on the the URI that's actually being displayed in the URL bar. Everything else has remained unchanged.

String changes made/needed

none

Flags: needinfo?(jh+bugzilla)
Attachment #9041241 - Flags: approval-mozilla-beta?

Jan, all 3 patches or just part 2?

Flags: needinfo?(jh+bugzilla)

All three.

Flags: needinfo?(jh+bugzilla)

Comment on attachment 9041241 [details]
Bug 1524857 - Part 2: Use display URI's base domain for domain highlighting. r?snorp

Fix for url display in some edge cases, OK to uplift for beta 7.

Attachment #9041241 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9041240 - Flags: approval-mozilla-beta+
Attachment #9041239 - Flags: approval-mozilla-beta+

Hi, verified as fixed on the latest version of Beta 66.0b7 and Nightly 67.0a1 (2019-02-12).
Devices:

  • OnePlus 5T (Android 9);
  • Samsung Galaxy S8 (Android 8.0).

Due to that, I'll mark this issue as verified, thanks.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.