Closed Bug 493467 Opened 12 years ago Closed 12 years ago

preserve allowDNSPrefetch and allowAuth and test for completeness

Categories

(Firefox :: Session Restore, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3.6a1

People

(Reporter: zeniko, Assigned: zeniko)

References

Details

Attachments

(2 files, 3 obsolete files)

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.
Attached patch fix (obsolete) — Splinter Review
Attachment #377947 - Flags: review?(dietrich)
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.
Flags: blocking-firefox3.5?
Attached patch one-liner and tests (obsolete) — Splinter Review
(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.
Assignee: nobody → zeniko
Status: NEW → ASSIGNED
Attachment #377982 - Flags: review?(dietrich)
Attachment #377947 - Attachment is obsolete: true
Attachment #377947 - Flags: review?(dietrich)
> 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".
Blocks: 492196
No longer depends on: 492196
Attachment #377982 - Flags: review?(dietrich) → review+
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
Attached patch for check-in (obsolete) — Splinter Review
Attachment #377982 - Attachment is obsolete: true
Keywords: checkin-needed
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?]
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 on attachment 378003 [details] [diff] [review]
for check-in

a191=beltzner
Attachment #378003 - Flags: approval1.9.1+
(forgot to minus it, but renominate if you can tell me why it should block release)
Flags: blocking-firefox3.5? → blocking-firefox3.5-
On the branch, we'll need to make sure that the late-compat interface added in bug 492196 is actually available.
The mozilla-central patch doesn't apply cleanly. :(
Keywords: checkin-needed
Attachment #378003 - Attachment is obsolete: true
Keywords: checkin-needed
Target Milestone: --- → Firefox 3.6a1
Checked in to m-c: http://hg.mozilla.org/mozilla-central/rev/66eb2c425c6a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Attachment #378003 - Flags: approval1.9.1+ → approval1.9.1-
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
Simon, http://hg.mozilla.org/releases/mozilla-1.9.1/rev/9d3d97f89d22 shows that this was checked in again against approvals; why?
Ignore comment 18: firebot was just late in notifying me, hg pushlog shows that it was backed out and stayed there.
Blocks: 524365
Comment on attachment 378401 [details] [diff] [review]
1.9.1 branch patch (for check-in)


Is this patch obsolete or waiting for approval?
(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).
You need to log in before you can comment on or make changes to this bug.