193623 - LXR uses internal-gopher-* image URLs in file browsing mode

Status: RESOLVED FIXED

(Webtools Graveyard :: MXR, defect)

Description

[Boris Zbarsky [:bzbarsky]]

These URLs are not supported by browsers other than Netscape and Mozilla (eg IE 
does not support them) and I'll be removing Mozilla's support for them in bug 
83774.  So it really would be nice if we were to stop using them.... (if we 
want to continue having these be Mozilla-only, we should switch to the 
appropriate resource:// urls instead).

Comment 1

[Malcolm Rowe]

This is really distracting if you're using any other browser.

I think all that's needed is to use the commented-out lines at
http://lxr.mozilla.org/mozilla/source/webtools/lxr/source#39

The images are already on lxr.m.o.

Comment 2

[Malcolm Rowe]

Should this go to gerv rather than endico, now?
(based on http://www.mozillazine.org/articles/article2925.html)

Comment 3

[Gervase Markham [:gerv]]

No; I do not own LXR.

Gerv

Comment 4

[Boris Zbarsky [:bzbarsky]]

Don't remove my tracking stuff, mmkay?

Comment 5

[timeless]

Attachment: draft (-uw) for review

Comment 6

[Malcolm Rowe]

Nice, and better than what I was suggesting.

It should use '301 Moved Permanently' rather than '307 Temporary Redirect', 
though, so the client can cache the response.

Comment 7

[timeless]

i don't understand the meaning of the different http responses, my concern is
that if someone actually does check in a file with one of those names that the
browser not have decided that it would never show the user that file.

307 is supposed to be cachable unless otherwise marked, so it should work.
if someone can explain to me that 301 would be safe for the case where someone
actually does stick a file by that name into the directory then i'd gladly use
it instead, i just don't think that it could.

Comment 8

[Bradley Baetz (:bbaetz)]

Why not just make lxr not generate those urls in teh first place, and instead
point to /icons/whatever.gif directly?

Comment 9

[Malcolm Rowe]

bbaetz: That was the fix I suggested.  timeless' fix means that browsers that 
support the internal-gopher-* images internally (NS1/2/3/4, Mozilla before bug 
83774 lands) won't bother to request the image at all - a neater solution, I 
think.

timeless: you're right, partly, but I'm not sure it's a real problem.

Actually, the response to 307 *isn't* cacheable unless you send a Cache-Control 
or Expires header (see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html). 
301 is cacheable by default. Of course, whether the image itself is cacheable 
depends on the headers for the second response.

However, 307 doesn't exist in HTTP/1.0. I think 301 is therefore a better 
choice, if only for that reason.

You're right that this would prevent the browser from accessing a file with the 
same name as one of the internal-gopher-* types, but current (NS/Mozilla) 
browsers don't make requests for those files anyway.  In any case, from what I 
recall, I thought your patch made LXR send the redirect regardless of whether 
the file existed?

In summary, I think we should go with timeless' patch. internal-gopher-* 
supporting browsers will see no change at all, and all other browsers will be 
transparently redirected to the correct image.

Comment 10

[Malcolm Rowe]

s/go with timeless' patch/go with timeless' patch (with 301 redirect)/

sorry for the spam.

Comment 11

[Bradley Baetz (:bbaetz)]

Given that mozilla soon won't support these images, I really don't think we 
need to care about the bandwidth of image requests to lxr from netscape4 
browsers, which are the only browsers which could take advantage of this.

Comment 12

[Malcolm Rowe]

Don't forget all the installed copies of Netscape 6.x, Mozilla 1.0.x/Netscape 
7.x, Mozilla 1.1 through (probably) 1.4 - they'll continue to take advantage of 
the internal-gopher-* images.  I don't think it's a particularly big deal, 
though, either way.

Comment 13

[Malcolm Rowe]

Attachment: Alternate minimal patch

The 'easy' way to fix this would be to do this.  I still think timeless' way is
neater, though.

Comment 14

[Alfred Kayser]

Mozilla 1.4 will be released soon (RSN), where these
'internal-gopher' are no longer supported. 

So, why not be consistent just do the right thing.
Use cachable images from l.m.o, with the right settings
so that standard compliant browsers work optimally, and that
old ones like NS4 still work.

Comment 15

[Bradley Baetz (:bbaetz)]

*** Bug 206948 has been marked as a duplicate of this bug. ***

Comment 16

[John Keiser (jkeiser)]

Comment on attachment 116188 [details] [diff] [review]
Alternate minimal patch

I rather prefer this patch.  We shouldn't penalize everyone else with redirects
and such for a non-standard extension that only we implement and that people
here don't even *like*.

Honestly, it's the sort of thing that could be tested on lxr.mozilla.org after
checkin and backed out if it doesn't work, but it needs to be tested
*somewhere*.

Comment 17

[Malcolm Rowe]

Comment on attachment 116188 [details] [diff] [review]
Alternate minimal patch

Anyone care to check this in? (webtools patches don't need sr, so this is ready
to go)

Comment 18

[Dave Miller [:justdave]]

timeless?  This is your bug...

Comment 19

[camson]

Attachment: test

test

Comment 20

[camson]

Attachment: test

test

Comment 21

[Michael Newton]

seems to be fixed

Comment 22

[Boris Zbarsky [:bzbarsky]]

Yeah, this is fixed.