URL bar: Consider showing full URL on tablets

VERIFIED FIXED in Firefox 47

Status

()

Firefox for Android
Awesomescreen
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: sebastian, Assigned: sebastian)

Tracking

unspecified
Firefox 48
All
Android
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox47 verified, firefox48 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
On tablets we have much more space to show the (full) URL. Spoofing as described in bug 1236431 might not be an issue here.
+1
(Assignee)

Comment 2

2 years ago
Created attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

Review commit: https://reviewboard.mozilla.org/r/36219/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36219/
Attachment #8722755 - Flags: review?(michael.l.comella)
(Assignee)

Updated

2 years ago
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
(In reply to Sebastian Kaspari (:sebastian) from comment #0)
> On tablets we have much more space to show the (full) URL. Spoofing as
> described in bug 1236431 might not be an issue here.

This is more questionable for 7" tablets in portrait mode – should we consider limiting by orientation and/or screen size?
Flags: needinfo?(alam)
Attachment #8722755 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

https://reviewboard.mozilla.org/r/36219/#review33879

lgtm pending UX review.
(Assignee)

Comment 5

2 years ago
(In reply to Michael Comella (:mcomella) from comment #3)
> (In reply to Sebastian Kaspari (:sebastian) from comment #0)
> > On tablets we have much more space to show the (full) URL. Spoofing as
> > described in bug 1236431 might not be an issue here.
> 
> This is more questionable for 7" tablets in portrait mode – should we
> consider limiting by orientation and/or screen size?

In SF we decided to show the full URL on all tablets and orientations. However it will always be possible to move the base domain off the screen with a long subdomain - if we show the full URL. The question is whether this looks suspicious to the user or if this could be used to trick the user into thinking the browser is connected to a different domain.
We have two - sometimes contradicting - goals here: Showing the user the location of the page and/or showing the user /who/ the browser is connected to. On phones we started to rank the "who" higher now. On tablets we try to do both but this can be hard.
Yeah, I think we should still continue to show the URL in the URL bar here since we have the tabs on top as well. 

I don't think there's much from the work we did on the mobile side that affects tablets immediately. So, as we discussed, we can probably continue as we are on Tablets unless we hear more feedback otherwise.
Flags: needinfo?(alam)
(Assignee)

Comment 7

2 years ago
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36219/diff/1-2/
(Assignee)

Comment 8

2 years ago
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

I'll re-request review because I changed the code: The previous patch did not restore the two-color URL on tablets. There's no reason to drop this functionality.

Additionally I started to reuse the ForegroundColorSpan for the certificate case (Your suggestion from bug 1249594) after seeing that we keep all the other spans too.
Attachment #8722755 - Flags: review+ → review?(michael.l.comella)
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

https://reviewboard.mozilla.org/r/36219/#review35203

::: mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java:146
(Diff revision 2)
> +        mUrlColor = new ForegroundColorSpan(ColorUtils.getColor(context, R.color.url_bar_urltext));
>          mBlockedColor = new ForegroundColorSpan(ColorUtils.getColor(context, R.color.url_bar_blockedtext));
> +        mDomainColor = new ForegroundColorSpan(ColorUtils.getColor(context, R.color.url_bar_domaintext));
> +        mPrivateDomainColor = new ForegroundColorSpan(ColorUtils.getColor(context, R.color.url_bar_domaintext_private));
> +        mCertificateOwnerColor = new ForegroundColorSpan(ColorUtils.getColor(context, R.color.affirmative_green));

nit: rename -> `*Span`. Ending the name with `*Color` makes me think these are ints!

::: mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java:309
(Diff revision 2)
> +    private void updateTitleFromFullURL(String url, String baseDomain,boolean isPrivate) {

nit: a name like, `updateAndColorTitleTitleFromFullURL` might be more descriptive.

nit: space after ,

::: mobile/android/base/java/org/mozilla/gecko/toolbar/ToolbarDisplayLayout.java:320
(Diff revision 2)
> +        index, index + baseDomain.length(), Spannable.SPAN_INCLUSIVE_INCLUSIVE);

nit: rb might be screwing with me, but this should be indented.
Attachment #8722755 - Flags: review?(michael.l.comella) → review+
(Assignee)

Comment 10

2 years ago
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36219/diff/2-3/
Attachment #8722755 - Attachment description: MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r?mcomella → MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella
(Assignee)

Comment 12

2 years ago
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

Approval Request Comment

[Feature/regressing bug #]: Displaying of base domain introduced in bug 1236431 (Firefox for Android 47).

[User impact if declined]: We decided to only show the base domain on mobile phones. On tablets we have more space and want to continue showing the full URL.

[Describe test coverage new/current, TreeHerder]: Local testing on a Nexus 9.

[Risks and why]: Low - This is more or less the same code we have been using before the patch in bug 1236431 landed.

[String/UUID change made/needed]: -
Attachment #8722755 - Flags: approval-mozilla-aurora?

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/386a3f51df04
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48

Updated

2 years ago
status-firefox47: --- → affected
Flags: qe-verify+
Comment on attachment 8722755 [details]
MozReview Request: Bug 1250671 - Show full URL in URL bar on tablets. r=mcomella

Seems like a good idea, taking it.
Attachment #8722755 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/ecdc38c5ea39
status-firefox47: affected → fixed

Comment 16

2 years ago
Verified as fixed in builds:
- 48.0a1 2016-03-10;
- 47.0a2 2016-03-11;
Device: Asus ZenPad 8 (Android 5.0.2).
Status: RESOLVED → VERIFIED
status-firefox47: fixed → verified
status-firefox48: fixed → verified
(Assignee)

Updated

2 years ago
Depends on: 1255767
(Assignee)

Updated

2 years ago
See Also: → bug 1268753

Comment 17

2 years ago
The following either A: breaks the way the EVSSL thing works or B: disables the showing of the name instead of the url in the urlbar.

'''security.ssl.enable_ocsp_stapling''' = '''false''' 
'''security.OCSP.enabled''' = '''0'''

And so FF doesn't grey out parts of the url & blocks the showing of the http:// part I also DISABLED both of these:
'''browser.urlbar.trimURLs'''
'''browser.urlbar.formatting.enabled'''
 
As you can see in my screenshot it no longer shows the name, it now shows the full URL.
(Assignee)

Updated

2 years ago
See Also: → bug 1271998
You need to log in before you can comment on or make changes to this bug.