Closed Bug 729495 Opened 13 years ago Closed 9 years ago

Hide http:// in URLs

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(blocking-fennec1.0 -)

RESOLVED WORKSFORME
Tracking Status
blocking-fennec1.0 --- -

People

(Reporter: pretzer, Unassigned)

References

Details

(Whiteboard: [lang=java][parity-desktop][parity-chrome][parity-stock])

Attachments

(3 files, 2 obsolete files)

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.
Whiteboard: uiwanted
Attached image Mockup of awesome screen (deleted) —
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.
Attachment #600663 - Flags: feedback?(madhava)
Attached image Original screenshot for comparison (deleted) —
Here is the original screenshot, so one can better compare it with the mockup. I think this would be a nice cleanup.
UX call
Keywords: uiwanted
Whiteboard: uiwanted
Comment on attachment 600663 [details] Mockup of awesome screen Requesting feedback from Ian, since Madhava is at MWC
Attachment #600663 - Flags: feedback?(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.
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.
Attachment #600663 - Flags: feedback?(madhava)
Attachment #600663 - Flags: feedback?(ibarlow)
Status: UNCONFIRMED → NEW
Ever confirmed: true
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → -
Assignee: nobody → ibarlow
unassigning myself, not sure there's anything I need to design for here.
Assignee: ibarlow → nobody
Only reason you were assigned was to get UX blessing on the change.
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:" ?
(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.)
Attached patch patch (obsolete) (deleted) — Splinter Review
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.
Assignee: nobody → bnicholson
Attachment #631577 - Flags: review?(mark.finkle)
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.
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.
Attached patch patch v2 (obsolete) (deleted) — Splinter Review
fixed end slash removal for non-http schemes
Attachment #631577 - Attachment is obsolete: true
Attachment #631577 - Flags: review?(mark.finkle)
Attachment #632815 - Flags: review?(mark.finkle)
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!
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.
(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).
(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.
(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. ;-)
(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://".
(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
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.
(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.
Oops, sorry - I misread your comment. Always showing the full URL will fix that.
Attached patch patch v3 (deleted) — Splinter Review
Rebased, and shows full URL in AwesomeScreen text input.
Attachment #632815 - Attachment is obsolete: true
Attachment #632815 - Flags: review?(mark.finkle)
Attachment #639946 - Flags: review?(mark.finkle)
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.
Attachment #639946 - Flags: review?(mark.finkle) → review+
(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!).
(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.
Do you still plan to fix the test issues and reland, Brian?
(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 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?
(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.
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.
Whiteboard: ui-hackathon
Whiteboard: ui-hackathon
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.
Unassigning myself since I have no immediate plans to work on this.
Assignee: bnicholson → nobody
(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.
Flags: needinfo?(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.
Flags: needinfo?(bnicholson)
Whiteboard: [lang=java][mentor=bnicholson]
Whiteboard: [lang=java][mentor=bnicholson] → [lang=java][mentor=bnicholson][parity-desktop][parity-chrome][parity-stock]
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
Welcome Taha, I am assigning the bug to you. Brian, can be your mentor and has mentioned some starting material in comment #40.
Assignee: nobody → tahalukmanji
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)
Flags: needinfo?(tahalukmanji)
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.
Great! I'll reassign this to you. See comment 40 for some starting tips.
Assignee: tahalukmanji → cjbarker
Flags: needinfo?(tahalukmanji)
Status: NEW → ASSIGNED
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.
Flags: needinfo?(cjbarker)
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
Mentor: bnicholson
Whiteboard: [lang=java][mentor=bnicholson][parity-desktop][parity-chrome][parity-stock] → [lang=java][parity-desktop][parity-chrome][parity-stock]
Unassigning due to inactivity; feel free to take this back if you'd like to work on it.
Assignee: cjbarker → nobody
Flags: needinfo?(cjbarker)
Mentor: bnicholson
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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: