URL Bar should color the domain differently from the rest of the URL

RESOLVED FIXED

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dhenein, Assigned: sleroux)

Tracking

unspecified
Other
iOS

Firefox Tracking Flags

(fxios+)

Details

(Whiteboard: noteworthy)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
As seen in the mockup here http://invis.io/FC25FA153 as well as Firefox on Android, we should color the url accordingly for better contrast and visual parsing.
tracking-fennec: ? → +
Duplicate of this bug: 1144841

Updated

3 years ago
tracking-fxios: --- → +
(Assignee)

Updated

3 years ago
Assignee: nobody → sleroux
(Assignee)

Comment 2

3 years ago
Created attachment 8621642 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/598

Set colors to be default dark grey/light grey.
Attachment #8621642 - Flags: review?(bnicholson)
Attachment #8621642 - Flags: feedback?(dhenein)
Comment on attachment 8621642 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/598

Looks good! I'm not sure we want to mess with the scheme display, though, as mentioned in the PR (looks like Darrin's mockup also includes "https://").
Attachment #8621642 - Flags: review?(bnicholson) → review+
(Assignee)

Comment 4

3 years ago
So I updated the patch to include changes in the PR comments and refactored out the logic for coloring the URL into extension methods on NSURL and NSAttributedString so I could wrap some of the behavior in tests. To color the host without the subdomains, I'm currently using the last two components of the host, joining them, and coloring that in hopes of excluding any subdomains. Would this work as a workaround? If not, I can look into using the tld.dat list to create special handling and wrap that in some additional tests if needed.
We don't want to break things like www.bbc.co.uk . I think thats why things got complex originally...
(Assignee)

Comment 6

3 years ago
Would it be difficult to emulate what Android does? I can probably port over whatever checking logic we do in Java over to Swift.
Android uses Gecko's code, so its basically the same as desktop.
(Assignee)

Updated

3 years ago
Attachment #8621642 - Attachment is obsolete: true
Attachment #8621642 - Flags: feedback?(dhenein)
(Assignee)

Comment 8

3 years ago
Created attachment 8623277 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/598

I managed to find http://mxr.mozilla.org/mozilla-central/source/netwerk/dns/nsEffectiveTLDService.cpp#224 in Gecko which is the code thats responsible for determining the public suffix/base domain for a given hostname using the public suffix list found here: https://publicsuffix.org/. I implemented the algorithm in Swift as an extension to NSURL so we can match the domain highlighting logic we have on Android/Desktop. I also included some basic test cases to confirm the algorithm's logic.
Attachment #8623277 - Flags: review?(sarentz)
Comment on attachment 8623277 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/598

Couple of regressions:

We show https:// again in front of the URL. (But not http://) Not sure if that was a design decision but before this patch we did not?

If you open https://www.apple.com/ only apple.com is darker grey. Is that intentional?

Opening a new tab shows the URL of the previously active tab in the location bar instead of "Search or enter address" - Not sure if that is this bug or the recent refactor of this url bar?
Comment on attachment 8623277 [details] [review]
PR https://github.com/mozilla/firefox-ios/pull/598

Code looks great. Nice to have that TLD code in the project, I think that will be useful for other things too. Just see my regression notes in the previous comment.

Are the performance test data (.xcbaseline) supposed to be committed? I don't know how that stuff works.
Attachment #8623277 - Flags: review?(sarentz) → review+
(Assignee)

Comment 11

3 years ago
> We show https:// again in front of the URL. (But not http://) Not sure if that was a design decision but before this patch we did not?

Just spoke with Darrin and he said to go with this behavior as it matches what we do on desktop

> If you open https://www.apple.com/ only apple.com is darker grey. Is that intentional?

Yup - Firefox Desktop does the same thing.

> Opening a new tab shows the URL of the previously active tab in the location bar instead of "Search or enter address" - Not sure if that is this bug or the recent refactor of this url bar?

This is actually a regression thats on master at the moment.
(Assignee)

Comment 12

3 years ago
See https://bugzilla.mozilla.org/show_bug.cgi?id=1175792 for URL regression bug.
(Assignee)

Comment 13

3 years ago
Should I wait on merging this until we fix the URL regression on master?
Flags: needinfo?(sarentz)
Nope lets merge it!
Flags: needinfo?(sarentz)
(Assignee)

Comment 15

3 years ago
Merged
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
tracking-fennec: + → ---
Whiteboard: noteworthy
Depends on: 1183353
You need to log in before you can comment on or make changes to this bug.