493467 - preserve allowDNSPrefetch and allowAuth and test for completeness

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Simon Bünzli]

I guess we need a better way of keeping up with the nsIDocShell:allow... flags, either by iterating through all of a docshell's property the first time we encounter one and take note of all attributes starting with allow, or by doing this in a test and verifying that SessionStore tracks all of them when disabled.

Comment 1

[Simon Bünzli]

Attachment: fix

Comment 2

[Boris Zbarsky [:bzbarsky]]

Should this be blocking bug 492196 (as in, a regression from it) instead of depending on it?

More to the point, should this be going on branch?  I plan to land that bug on branch tonight.

Comment 3

[Simon Bünzli]

Attachment: one-liner and tests

(In reply to comment #2)
> Should this be blocking bug 492196 (as in, a regression from it)

It's not a regression in the strict sense (no functionality was lost), we just have to cater for newly added functionality.

> More to the point, should this be going on branch?

It should, if at all possible, for consistency reasons.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> It's not a regression in the strict sense (no functionality was lost)

Well...  it is in that we now have incorrect behavior where before that patch we had correct behavior, no?  And further, the incorrect behavior may have security implications.  So while it's not a "regression" it should be blocking that bug in the sense of "can't ship this until this other bug is also fixed".

Comment 5

[Dietrich Ayala (:dietrich)]

Comment on attachment 377982 [details] [diff] [review]
one-liner and tests

> 
> /*
>-docShell capabilities to (re)store
>+docShell capabilities to (re)store (please keep in sync!)
> Restored in restoreHistory()
> eg: browser.docShell["allow" + aCapability] = false;
> */

please add a note about how to keep it in sync - either point to files, or a documentation URL.

r=me otherwise

Comment 6

[Simon Bünzli]

Attachment: for check-in

Comment 7

[[e-mail]]

Comment on attachment 378003 [details] [diff] [review]
for check-in

>+XXX keep these in sync with all the attributes starting
>+    with "allow" in /docshell/base/nsIDocShell.idl
[Maybe the nsDocShell should start storing these as a bitfield?]

Comment 8

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

Can I get a quick overview of why this blocks vs. why it should be approved? I think I'm happy to approve, just don't know if I'd block on this if it causes a regression or otherwise needs to be backed out.

Comment 9

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

Comment on attachment 378003 [details] [diff] [review]
for check-in

a191=beltzner

Comment 10

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

(forgot to minus it, but renominate if you can tell me why it should block release)

Comment 11

[Simon Bünzli]

Attachment: 1.9.1 branch patch (for check-in)

On the branch, we'll need to make sure that the late-compat interface added in bug 492196 is actually available.

Comment 12

[Joe Drew (not getting mail)]

The mozilla-central patch doesn't apply cleanly. :(

Comment 13

[Simon Bünzli]

Attachment: for check-in (unbitrotted)

Comment 14

[Joe Drew (not getting mail)]

Checked in to m-c: http://hg.mozilla.org/mozilla-central/rev/66eb2c425c6a

Comment 15

[Joe Drew (not getting mail)]

Had to refresh the patch for Makefile.in.

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9d3d97f89d22

Comment 16

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

Comment on attachment 378003 [details] [diff] [review]
for check-in

removing approval, asking joe to back this out

If you'd like, please renom for 3.5.1, but doesn't feel like there's a real gain to get this into 3.5.0

Comment 17

[Joe Drew (not getting mail)]

backed out http://hg.mozilla.org/releases/mozilla-1.9.1/rev/750456e89148

Comment 18

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

Simon, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9d3d97f89d22 shows that this was checked in again against approvals; why?

Comment 19

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

Ignore comment 18: firebot was just late in notifying me, hg pushlog shows that it was backed out and stayed there.

Comment 20

[Serge Gautherie (:sgautherie)]

Comment on attachment 378401 [details] [diff] [review]
1.9.1 branch patch (for check-in)


Is this patch obsolete or waiting for approval?

Comment 21

[Simon Bünzli]

(In reply to comment #20)
It's obsolete (landed on Trunk pre 3.6 branching in comment #14 and was denied for Firefox 3.5 in comment #16).