475069 - nsNavHistory::AddDocumentRedirect doesn't handle INTERNAL redirects

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Ignore INTERNAL redirects

AddDocumentRedirect() doens't check for or handle INTERNAL redirects. It should likely ignore them. I ran into this when trying land the PAC patch from bug 235853 which causes a INTERNAL redirects to happen much more frequently. The INTERNAL redirects seem to have been breaking toolkit/components/places/tests/test_bug_411966.html.

Comment 1

[Marco Bonardo [:mak]]

Comment on attachment 358465 [details] [diff] [review]
Ignore INTERNAL redirects


>+  // ignore internal redirects
>+  if (aFlags & nsIChannelEventSink::REDIRECT_INTERNAL)
>+      return NS_OK;
>+

nit: please fix indentation on the return
looks good otherwise

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Ignore INTERNAL redirects w/ indentation fixed.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 358738 [details] [diff] [review]
Ignore INTERNAL redirects w/ indentation fixed.

Someone like mconnor (or someone else who knows what this code is trying to do) is a better sr here.  That's assuming this patch needs sr at all.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

It doesn't need SR.

Comment 5

[Joe Drew (not getting mail)]

http://hg.mozilla.org/mozilla-central/rev/ace745f31c3e

Jeff, if you want this into 1.9.1, you'll need to ask for approval1.9.1.

Comment 6

[Marco Bonardo [:mak]]

Comment on attachment 358738 [details] [diff] [review]
Ignore INTERNAL redirects w/ indentation fixed.

askign approval 1.9.1, should have no risk and avoid us to retain useless (for Places) redirect data.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 358738 [details] [diff] [review]
Ignore INTERNAL redirects w/ indentation fixed.

a191=beltzner

Comment 8

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d2cce66f5097