Lin Clark [:linclark] Reporter
	Description • 8 years ago

Urls seem to be handled in different ways

- window shows full url: http://example.com/
- document doesn't include protocol: www.facebook.com
- stylesheet only includes file name: contentSearchUI.css

I believe all of these (and any others) should be full urls.

	[:Towkir] Ahmed
	Comment 1 • 8 years ago

Hi Lin, 
I would like to work on this,
It would be great if you could tell me where to start with

	Lin Clark [:linclark] Reporter
	Comment 2 • 8 years ago

Sorry, I just realized this bug isn't ready yet. We have a few issues with patches that are going to land soon which need to be done before this. I've attached that as a blocker.

	Dalimil Hajek [:dalimil] Assignee
	Comment 3 • 8 years ago

Test for the relevant rep modules have already been created so this bug wasn't blocked anymore and also had a relatively simple solution. 

Push to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=668af6412833

	Lin Clark [:linclark] Reporter
	Comment 4 • 8 years ago

Comment on attachment 8774705 [details] [diff] [review]
Bug1284842.patch - rev1

Review of attachment 8774705 [details] [diff] [review]:
-----------------------------------------------------------------

Rather than using crop string, we should be passing this through a URL function. It may be that we need to update the url function to process it properly.

	Dalimil Hajek [:dalimil] Assignee
	Comment 5 • 8 years ago

(In reply to Lin Clark [:linclark] from comment #4)

> Rather than using crop string, we should be passing this through a URL
> function. It may be that we need to update the url function to process it
> properly.
There are a few URL related functions in rep-utils but they are mostly used for splitting. I thought we just want to display the url as is (possibly cropped)? Or do you want me to just create a simple function parseURLToDisplay(urlText) in rep-utils that for now would only call cropString() - but would provide an abstraction to change it later? 
BTW, I also checked object-with-url.js and it doesn't even crop the url.

	Lin Clark [:linclark] Reporter
	Comment 6 • 8 years ago

I think it would be best to compile a list here of all of the places in reps where urls are shown and what function is used to show the URL. We can make a better decision once we have that information.

	Dalimil Hajek [:dalimil] Assignee
	Comment 7 • 8 years ago

OK, here are the ones that I was able to find (looking at 'master' and ignoring the new patch):
window.js -> cropString(grip.preview.url)
document.js -> getFileName(grip.preview.location)
stylesheet.js -> getFileName(grip.preview.url)
object-with-url.js -> displays 'grip.preview.url' (without any cropping)
rep-utils.js -> contain URL parsing and splitting methods that are used in reps

	Dalimil Hajek [:dalimil] Assignee
	Comment 8 • 8 years ago

Honza, could you take a look at this? I feel this has got stuck...

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 9 • 8 years ago

It's great you are working on this!

I was quickly discussing this with Lin and we agree that there should be a function that's used to prepare the URL string for rendering. Just like you are mentioning in comment #5. Implementation of that function can be simple cropping (or some kind of smarter URL cropping?) and it should be used at all the places you summarized in comment #7. The function will be nice abstraction and so, we can improve it later if needed.

I can review the patch.

Honza

	Dalimil Hajek [:dalimil] Assignee
	Comment 10 • 8 years ago

OK, I created the new function in rep-utils.

Push to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ab6641ecfbb

	Patrick Brosset <:pbro>
	Comment 11 • 8 years ago

[good first bug] whiteboard -> keyword mass change

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 12 • 8 years ago

Comment on attachment 8781487 [details] [diff] [review]
Bug1284842.patch - rev2

Review of attachment 8781487 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me now!

R+ assuming the try is green


Honza

	Pulsebot
	Comment 13 • 8 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/1f04326ada82
Reps: Handle URLs uniformly. r=Honza

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/1f04326ada82