mxr goes to blank page with redirect warnings enabled

ASSIGNED
Assigned to

Status

Webtools
MXR
ASSIGNED
9 years ago
7 years ago

People

(Reporter: Akkana Peck, Assigned: timeless)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
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?
(Assignee)

Comment 2

9 years ago
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
Assignee: nobody → timeless
Status: NEW → ASSIGNED

Comment 3

7 years ago
Created attachment 514087 [details] [diff] [review]
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.
Attachment #514087 - Flags: review?(iann_bugzilla)
Attachment #514087 - Flags: feedback?(bzbarsky)

Comment 4

7 years ago
Comment on attachment 514087 [details] [diff] [review]
Proposed patch

Well it certain hacks around the problem, does it work any better as a doorhanger?
Attachment #514087 - Flags: review?(iann_bugzilla) → review+
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?
Attachment #514087 - Flags: feedback?(bzbarsky) → feedback-

Comment 6

7 years ago
(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...
If all you need is "really soon", shouldn't you just do a 0ms XPCOM timer?

Comment 8

7 years ago
Created attachment 520558 [details] [diff] [review]
Really soon

I thought a setTimeout would probably be soon enough.
Attachment #514087 - Attachment is obsolete: true
Attachment #520558 - Flags: review?(iann_bugzilla)

Comment 9

7 years ago
Comment on attachment 520558 [details] [diff] [review]
Really soon

Seems less hacky and still does the job.
Attachment #520558 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 520558 [details] [diff] [review]
Really soon

Pushed changeset 30a246100173 to comm-central.
You need to log in before you can comment on or make changes to this bug.