516441 - mxr goes to blank page with redirect warnings enabled

Status: RESOLVED WONTFIX

(Webtools Graveyard :: MXR, defect)

Description

[Akkana Peck]

Enable redirect warnings (Preferences, Advanced, "Warn me when web sites try to redirect or reload the page").

Go to http://mxr.mozilla.org/mozilla1.9.1/ and search for a string.

It loads something, there's a brief flash at the top like it's trying to show a redirect warning, but no warning bar actually appears and I end up at a blank page.

The redirect warning seems to work for other websites; I'm not sure what's special about mxr.

Firefox Shiretoko (Ubuntu) 3.5.2.

Comment 1

[timeless]

http://mxr-test.konigsberg.mozilla.org/mxr-test/source/search?mark=621-628&rev=590dd67e0c48#609
http://mxr-test.konigsberg.mozilla.org/mxr-test/source/find?mark=231-233&rev=590dd67e0c48#208

the page we load is zero bytes long :) -- that could be the problem.

I'm willing to fix MXR if someone provides a suggestion, I suppose I can just provide a link to the page I'm going to visit....

But even doing that didn't seem to fix Firefox. I still got no warnings some of the times. and now i'm getting redirected without warning, does "allow" stop caring at some point?

Comment 2

[timeless]

i filed bug 516678 against firefox. i'm unlikely to fix this without:
1. firefox being fixed :)
2. someone suggesting text they actually like for this page -- this is the primary reason I never added text, i could never come up with reasonable text, it's incredibly awkward.

you can try:
http://mxr-test.konigsberg.mozilla.org/mozilla/search?find=this&find=js&tree=mozilla-central

Comment 3

[[e-mail]]

Attachment: Proposed patch

We seem to get the onRefreshAttempted before the onLocationChange in this case; this patch hacks around it by allowing 300ms for the location to change.

Comment 4

[Ian Neal]

Comment on attachment 514087 [details] [diff] [review]
Proposed patch

Well it certain hacks around the problem, does it work any better as a doorhanger?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 514087 [details] [diff] [review]
Proposed patch

Shouldn't you just check whether you got the location change yet?  What makes sure that 300ms will be enough?

Comment 6

[[e-mail]]

(In reply to comment #5)
> Shouldn't you just check whether you got the location change yet?
What do you mean by "yet"? This callback can be fired by an HTTP refresh, in which case the location is about to change, or by a meta header, in which case it has, but the API does not indicate which.

>  What makes sure that 300ms will be enough?
The HTTP refresh occurs at the beginning of CreateContentViewer and the associated location change at the end, so basically all I need is "really soon".

300ms might actually be too much in case script changes the location...

Comment 7

[Boris Zbarsky [:bzbarsky]]

If all you need is "really soon", shouldn't you just do a 0ms XPCOM timer?

Comment 8

[[e-mail]]

Attachment: Really soon

I thought a setTimeout would probably be soon enough.

Comment 9

[Ian Neal]

Comment on attachment 520558 [details] [diff] [review]
Really soon

Seems less hacky and still does the job.

Comment 10

[[e-mail]]

Comment on attachment 520558 [details] [diff] [review]
Really soon

Pushed changeset 30a246100173 to comm-central.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

mxr is gone, mass closing.
https://searchfox.org/ is a much better alternative.