729495 - Hide http:// in URLs

Status: RESOLVED WORKSFORME

(Firefox for Android Graveyard :: General, defect)

Description

[Peter Retzer (:pretzer)]

Desktop Firefox is hiding the 'http://' protocol and the trailing '/' in URLs since Version 7.0 (bug 665580).
Firefox (and Chrome/Opera) users are used to it now and Fennec should therefore follow this behaviour.

Comment 1

[Peter Retzer (:pretzer)]

Attachment: Mockup of awesome screen

I created a quick mockup of how this will look like on awesome screen, for the UX team to comment on and wether this is wanted or not. 
The correct way would be, to request UI-Review from the UX-Account, I think, but I don't seem to have the rights to do this. So I just requested feedback from Madhava, I hope that's okay.

Comment 2

[Peter Retzer (:pretzer)]

Attachment: Original screenshot for comparison

Here is the original screenshot, so one can better compare it with the mockup. I think this would be a nice cleanup.

Comment 3

[Aaron Train [:aaronmt]]

UX call

Comment 4

[Peter Retzer (:pretzer)]

Comment on attachment 600663 [details]
Mockup of awesome screen

Requesting feedback from Ian, since Madhava is at MWC

Comment 5

[Ian Barlow (:ibarlow)]

Hi there, I agree it makes sense to hide the "http://" and trailing "/", as long as we make sure to still display "https://" wherever it is used.

Comment 6

[Peter Retzer (:pretzer)]

Thanks, Ian! 
Another implementation note, besides always showing "https://", would be that on desktop when copying a URL, Firefox always copies the "whole" URL including the protocol to the clipboard.

Comment 7

[Ian Barlow (:ibarlow)]

unassigning myself, not sure there's anything I need to design for here.

Comment 8

[Kevin Brosnan [Ex-Mozilla]]

Only reason you were assigned was to get UX blessing on the change.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

We only show "http://" when you are editing the URL. Why would we hide it then? We show the title in the URLbar unless there is no title in which case we show the URL.

It's this last case (no title to show in the URLBar) that I would be OK with hiding the "http://". What about other schemes though? "about:", "chrome:", "ftp:" and "https:" ?

Comment 10

[Peter Retzer (:pretzer)]

(In reply to Mark Finkle (:mfinkle) from comment #9)
> We only show "http://" when you are editing the URL.

We also show it on every awesomescreen entry (Top Sites, Bookmarks, History), sometimes even twice for one entry, when the URL is also the title of that entry. That's the case where this change would bring the most benefit in my eyes... less clutter on awesomescreen.

For other schemes, I'd recommend to follow the desktop implementation here, which I think is this: 
Only hide "http:" and leave all other protocols/schemes as they are ("https:", "ftp:", etc.)

Comment 11

[Brian Nicholson (:bnicholson)]

Attachment: patch

This patch shows "pretty" URLs wherever they are displayed to the user. If copying the entire URL in the AwesomeScreen, the actual URL is copied to the clipboard.

Comment 12

[Madhava Enros [:madhava]]

I agree with the only hide http:// and leave in anything "unusual" like https, ftp, about,  etc. (what's proposed in comment 10).

Including the protocol on copy definitely makes sense.

This is what desktop Chrome does, and it doesn't seem crazy to me. People who are used to entering the http:// should be able to continue to do so and not have that break anything.

Comment 13

[Brian Nicholson (:bnicholson)]

This patch does 2 things: 1) remove the "http://" prefix, and 2) remove the trailing "/" on "root" URLs that don't have any subdirectories. This is the behavior I observed on desktop.

For example:
- http://www.google.com/ -> www.google.com
- http://mozilla.org/en-US/ -> mozilla.org/en-US/
- https://www.google.com/ -> https://www.google.com

Build available here:
http://dl.dropbox.com/u/35559547/fennec-no-http.apk

This is applied everywhere - in about:home tabs from last time, top sites (for those that don't have a title), the AwesomeScreen URL bar, and entries in the AwesomeScreen. If the entire URL is copied, the "unpretty" URL is put in the clipboard; otherwise, only the selection is copied.

Comment 14

[Brian Nicholson (:bnicholson)]

Attachment: patch v2

fixed end slash removal for non-http schemes

Comment 15

[Peter Retzer (:pretzer)]

Brian, since this is not reviewed yet, can you take a look at bug 768181 and see if you want to add that to the patch here or if we want to leave it as a followup? Anyways, thanks for your work on this already!

Comment 16

[Mark Finkle (:mfinkle) (use needinfo?)]

After testing the APK, I see that we are not using the "http://" when I tap on the awesomebar to edit the URL. I think we want to show the "http://" when editing the URL.

I mentioned this to Ian on IRC and he expected the same, IIRC.

Comment 17

[Peter Retzer (:pretzer)]

(In reply to Mark Finkle (:mfinkle) from comment #16)
> After testing the APK, I see that we are not using the "http://" when I tap
> on the awesomebar to edit the URL. I think we want to show the "http://"
> when editing the URL.
> 
> I mentioned this to Ian on IRC and he expected the same, IIRC.

Out of curiosity, what kind of use case do you think about that requires the "http://" to be shown while editing? 
I can't think of anything, except maybe when you want to change to protocol (like from "http:" to "ftp:") for the currently displayed address, which is rather an edge case in my eyes.
For the record Chrome for Android, Opera Mobile and Desktop Firefox do not show the "http://" while editing the address.
Android stock browser seems to be doing what you propose, Mark (generally hiding the "http://" and only showing it while editing the address).

Comment 18

[Kartikaya Gupta (email:[e-mail])]

(In reply to pretzer from comment #17)
> Out of curiosity, what kind of use case do you think about that requires the
> "http://" to be shown while editing? 

I often want to change the URL from http:// to https:// and would sorely miss being able to do so.

Comment 19

[Peter Retzer (:pretzer)]

(In reply to Kartikaya Gupta (:kats) from comment #18) 
> I often want to change the URL from http:// to https:// and would sorely
> miss being able to do so

I'd still call that an edge case, but that might just be me. :-) The most important part of this patch is the cleaner awesomescreen anyway in my eyes. And I hope it will make the cut to 16. ;-)

Comment 20

[Brian Nicholson (:bnicholson)]

(In reply to Kartikaya Gupta (:kats) from comment #18)
> (In reply to pretzer from comment #17)
> > Out of curiosity, what kind of use case do you think about that requires the
> > "http://" to be shown while editing? 
> 
> I often want to change the URL from http:// to https:// and would sorely
> miss being able to do so.

FWIW, with this patch, you'd still be able to prepend "https://" to the URL, like in desktop. Though I imagine you mean it's convenient to simply add the "s" to the existing "http://".

Comment 21

[Kartikaya Gupta (email:[e-mail])]

(In reply to Brian Nicholson (:bnicholson) from comment #20)
> FWIW, with this patch, you'd still be able to prepend "https://" to the URL,
> like in desktop. Though I imagine you mean it's convenient to simply add the
> "s" to the existing "http://".

While it is convenient to just be able to add an "s", I would be ok with being allowed to prepend "https://" to the URL manually. As a user I'd probably have been confused the first time I did that though, unsure if it would end up trying to fetch http://https://domain.com

Comment 22

[Kartikaya Gupta (email:[e-mail])]

Also, another reason for displaying the protocol when editing the URL is so that when you do a "copy" it behaves in a WYSIWYG manner. I hate it when I copy a url from a browser that doesn't display the http:// and then when I paste it there is magically a http:// in it.

Comment 23

[Brian Nicholson (:bnicholson)]

(In reply to Kartikaya Gupta (:kats) from comment #22)
> Also, another reason for displaying the protocol when editing the URL is so
> that when you do a "copy" it behaves in a WYSIWYG manner. I hate it when I
> copy a url from a browser that doesn't display the http:// and then when I
> paste it there is magically a http:// in it.

Yeah, this takes care of that (see comment 11). But I think we're changing this to always show the full URL in the edit box, so these won't be concerns anyway.

Comment 24

[Brian Nicholson (:bnicholson)]

Oops, sorry - I misread your comment. Always showing the full URL will fix that.

Comment 25

[Brian Nicholson (:bnicholson)]

Attachment: patch v3

Rebased, and shows full URL in AwesomeScreen text input.

Comment 27

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 639946 [details] [diff] [review]
patch v3

I don't like this change, but I'll concede because others seem to like it. I don't like needing to "fix" the URL in so many places in the code. I'm sure we'll be breaking this at some point in the near future.

Comment 28

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/751aa72f61a2

Comment 29

[Ed Morley [:emorley]]

Backed out for robocop failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=751aa72f61a2

https://hg.mozilla.org/integration/mozilla-inbound/rev/01a48ccbd15e

Comment 30

[Peter Retzer (:pretzer)]

(In reply to Ed Morley [:edmorley] from comment #29)
> Backed out for robocop failures:
> https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=751aa72f61a2

Any ideas what's causing these, Brian? (I have no experience in reading those TBPL reports, sorry!).

Comment 31

[Brian Nicholson (:bnicholson)]

(In reply to pretzer from comment #30)
> (In reply to Ed Morley [:edmorley] from comment #29)
> > Backed out for robocop failures:
> > https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=751aa72f61a2
> 
> Any ideas what's causing these, Brian? (I have no experience in reading
> those TBPL reports, sorry!).

We check for the full URL in several UI test cases, so we'll have to update all of them to check for the "pretty" URL instead.

Comment 32

[Peter Retzer (:pretzer)]

Do you still plan to fix the test issues and reland, Brian?

Comment 33

[Brian Nicholson (:bnicholson)]

(In reply to pretzer from comment #32)
> Do you still plan to fix the test issues and reland, Brian?

I'll take a look this week. I don't think this is a very high priority bug, though, so I can't guarantee this will land soon if the changes are non-trivial for whatever reason.

Comment 34

[Peter Retzer (:pretzer)]

Comment on attachment 639946 [details] [diff] [review]
patch v3

I'm curious: 
Will the URLs that show up in the Android global search (now that bug 786029 landed) also show up as "prettyURLs" with your current patch? Or would that need further adjustment?

Comment 35

[Brian Nicholson (:bnicholson)]

(In reply to Peter Retzer (:pretzer) from comment #34)
> Comment on attachment 639946 [details] [diff] [review]
> patch v3
> 
> I'm curious: 
> Will the URLs that show up in the Android global search (now that bug 786029
> landed) also show up as "prettyURLs" with your current patch? Or would that
> need further adjustment?

Nope, that's another change we'll need to make.

Comment 36

[Peter Retzer (:pretzer)]

For the record:
Bug 817739 will add the URL to synced tabs entries, so those will need to be made pretty as well, when this bug receives attention again.

Comment 37

[Peter Retzer (:pretzer)]

Anyone who picks this up again in the future might want to take a look at bug 857165 first. There's a first place that's showing a 'prettified' URL in product now and an updated patch here should probably share some code with that.

Comment 38

[Brian Nicholson (:bnicholson)]

Unassigning myself since I have no immediate plans to work on this.

Comment 39

[Peter Retzer (:pretzer)]

(In reply to Brian Nicholson (:bnicholson) from comment #38)
> Unassigning myself since I have no immediate plans to work on this.

Brian, would you be willing to mentor someone to fix this bug? Now that the about:home revamp landed I think it's a good time to get this going again. I'd try to fix it myself but unfortunately I lack time, sufficient hardware and probably also the coding skills to do so atm.

Comment 40

[Brian Nicholson (:bnicholson)]

The patch for the fix itself shouldn't be too hard; updating all of the test cases to work with these changes, however, will likely take some effort.

For anyone taking this bug, I recommend first getting Robocop tests running and passing before making any changes. See: https://wiki.mozilla.org/Auto-tools/Projects/Robocop

After that, I would make the necessary changes (see the old attached patch for a good starting point), see which tests fail after the change, and then update them accordingly.

Comment 41

[Taha Lukmanji]

Hi guys, is anyone is looking at this bug or feature request? If not, can someone assign this to me? I would like to work on this defect. Also I will need a mentor as I am not a Firefox yet? Thanks. Can't guarantee when this would be fixed but doesn't sound like a very difficult defect to solve

Comment 42

[Aaron Train [:aaronmt]]

Welcome Taha,

I am assigning the bug to you. Brian, can be your mentor and has mentioned some starting material in comment #40.

Comment 43

[Peter Retzer (:pretzer)]

Hi Taha, it's been a while since you commented, so I wanted to ask if you were able to make any progress here? Did you manage to build Firefox for Android yet? 
If you need any help going forward don't hesitate to ask questions here or in the #mobile channel on Mozilla's IRC server (irc.mozilla.org)

Comment 44

[CJ Barker]

Aaron, Peter, et al,

If Taha is MIA I don't mind picking this up and knocking it out for you all.  Feel free to assign this to me if it's gone stale.

I've already knocked out my first patch and working build of Fennec.

Comment 45

[Brian Nicholson (:bnicholson)]

Great! I'll reassign this to you. See comment 40 for some starting tips.

Comment 46

[Peter Retzer (:pretzer)]

Hi CJ, have you been able to make any progress here? Like I said to Taha before, please reach out to the folks on IRC (#mobile) or to Brian (mentor) here if you have any questions.

Comment 47

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Just to note, if this patch changes the title of the page during loading (which is initially "http://..."), it will break robocop tests based on UITest. See [1]. The regex that determines if a page is considered loading needs to be updated [2] (if possible, otherwise, we may need a new hack or a proper fix - feel free to needinfo me if you need help).

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/helpers/WaitHelper.java?rev=b46d73d9f715#157
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/helpers/WaitHelper.java?rev=b46d73d9f715#135

Comment 48

[Brian Nicholson (:bnicholson)]

Unassigning due to inactivity; feel free to take this back if you'd like to work on it.

Comment 49

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We do this already for displaying the url, but not for editing the url (which maybe we should argue is desired – it's nice to add an "s" to http rather than having to type "http://". Thus WORKSFORME.

If hiding http:// is desired for editing mode, someone can file a new bug and we can start the discussion again there.