Closed
Bug 729495
Opened 13 years ago
Closed 9 years ago
Hide http:// in URLs
Categories
(Firefox for Android Graveyard :: General, defect)
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.
Updated•13 years ago
|
Whiteboard: uiwanted
Reporter | ||
Comment 1•13 years ago
|
||
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)
Reporter | ||
Comment 2•13 years ago
|
||
Here is the original screenshot, so one can better compare it with the mockup. I think this would be a nice cleanup.
Reporter | ||
Comment 4•13 years ago
|
||
Comment on attachment 600663 [details]
Mockup of awesome screen
Requesting feedback from Ian, since Madhava is at MWC
Attachment #600663 -
Flags: feedback?(ibarlow)
Comment 5•13 years ago
|
||
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.
Reporter | ||
Comment 6•13 years ago
|
||
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.
Reporter | ||
Updated•13 years ago
|
Attachment #600663 -
Flags: feedback?(madhava)
Attachment #600663 -
Flags: feedback?(ibarlow)
Reporter | ||
Updated•13 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•13 years ago
|
blocking-fennec1.0: --- → ?
Updated•13 years ago
|
blocking-fennec1.0: ? → -
Updated•13 years ago
|
Assignee: nobody → ibarlow
Comment 7•13 years ago
|
||
unassigning myself, not sure there's anything I need to design for here.
Assignee: ibarlow → nobody
Comment 8•13 years ago
|
||
Only reason you were assigned was to get UX blessing on the change.
Comment 9•13 years ago
|
||
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:" ?
Reporter | ||
Comment 10•13 years ago
|
||
(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•12 years ago
|
||
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)
Comment 12•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
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)
Reporter | ||
Comment 15•12 years ago
|
||
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•12 years ago
|
||
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.
Reporter | ||
Comment 17•12 years ago
|
||
(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•12 years ago
|
||
(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.
Reporter | ||
Comment 19•12 years ago
|
||
(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•12 years ago
|
||
(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•12 years ago
|
||
(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•12 years ago
|
||
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•12 years ago
|
||
(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•12 years ago
|
||
Oops, sorry - I misread your comment. Always showing the full URL will fix that.
Comment 25•12 years ago
|
||
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 27•12 years ago
|
||
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+
Comment 28•12 years ago
|
||
Keywords: uiwanted
Target Milestone: --- → Firefox 16
Comment 29•12 years ago
|
||
Backed out for robocop failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=751aa72f61a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/01a48ccbd15e
Target Milestone: Firefox 16 → ---
Reporter | ||
Comment 30•12 years ago
|
||
(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•12 years ago
|
||
(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.
Reporter | ||
Comment 32•12 years ago
|
||
Do you still plan to fix the test issues and reland, Brian?
Comment 33•12 years ago
|
||
(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.
Reporter | ||
Comment 34•12 years ago
|
||
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•12 years ago
|
||
(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.
Reporter | ||
Comment 36•12 years ago
|
||
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.
Reporter | ||
Updated•12 years ago
|
Whiteboard: ui-hackathon
Updated•12 years ago
|
Whiteboard: ui-hackathon
Reporter | ||
Comment 37•12 years ago
|
||
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•11 years ago
|
||
Unassigning myself since I have no immediate plans to work on this.
Assignee: bnicholson → nobody
Reporter | ||
Comment 39•11 years ago
|
||
(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)
Comment 40•11 years ago
|
||
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]
Reporter | ||
Updated•11 years ago
|
Whiteboard: [lang=java][mentor=bnicholson] → [lang=java][mentor=bnicholson][parity-desktop][parity-chrome][parity-stock]
Comment 41•11 years ago
|
||
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•11 years ago
|
||
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
Reporter | ||
Comment 43•11 years ago
|
||
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)
Comment 44•11 years ago
|
||
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•11 years ago
|
||
Great! I'll reassign this to you. See comment 40 for some starting tips.
Assignee: tahalukmanji → cjbarker
Flags: needinfo?(tahalukmanji)
Reporter | ||
Comment 46•11 years ago
|
||
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)
Comment 47•11 years ago
|
||
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
Assignee | ||
Updated•10 years ago
|
Mentor: bnicholson
Whiteboard: [lang=java][mentor=bnicholson][parity-desktop][parity-chrome][parity-stock] → [lang=java][parity-desktop][parity-chrome][parity-stock]
Comment 48•10 years ago
|
||
Unassigning due to inactivity; feel free to take this back if you'd like to work on it.
Assignee: cjbarker → nobody
Flags: needinfo?(cjbarker)
Updated•9 years ago
|
Mentor: bnicholson
Comment 49•9 years ago
|
||
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
Assignee | ||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•