Closed
Bug 283201
Opened 19 years ago
Closed 19 years ago
Entering/leaving security warnings when staying on https
Categories
(Core Graveyard :: Security: UI, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: dveditz, Assigned: darin.moz)
References
Details
(Keywords: fixed-aviary1.0.1, fixed1.7.6, regression, Whiteboard: [sg:fix])
Attachments
(2 files)
3.55 KB,
patch
|
Details | Diff | Splinter Review | |
998 bytes,
patch
|
jst
:
review+
jst
:
superreview+
asa
:
approval-aviary1.0.1+
asa
:
approval1.7.6+
|
Details | Diff | Splinter Review |
Regression from bug 258048, surfing from secure site to secure site (e.g. click the "next" link in a list of bugzilla bugs) you get the warning dialogs for leaving/entering secure state. Rather than reopen ancient bug 154084 I'm filing this to address the regression as a clean state.
Reporter | ||
Updated•19 years ago
|
Flags: blocking1.7.6+
Flags: blocking-aviary1.0.1+
Whiteboard: [sg:fix]
Assignee | ||
Comment 1•19 years ago
|
||
I cannot reproduce this bug on the trunk. It only seems to happen on the branch.
Comment 2•19 years ago
|
||
nsSecureBrowserUIImpl::EvaluateAndUpdateSecurityState if (!channel) mNewToplevelSecurityState = nsIWebProgressListener::STATE_IS_INSECURE; That's switching the state and causing the additional transition warnings. But we must change the root of the problem. The OnLocationChange method is being called with additional request objects. I have no idea what kind of requests are being passed in there, but obviously, it must be items that don't have a channel. I propose we re-work the patch in bug 258048. Before the patch, calling OnLocationChange only triggered state changes for wyciwig requests. Instead of doing it for everything, let's do it only if: it's wyciwyg or it has a channel. So, I propose to revert the patch in bug 258048, and apply the patch I will attach in a minute.
Comment 3•19 years ago
|
||
Comment 4•19 years ago
|
||
The patch looks big, but all it does is: - revert 258048 - add the following small change: --- src/nsSecureBrowserUIImpl.cpp-before-2580248 2005-02-23 00:09:44.601874208 +0100 +++ src/nsSecureBrowserUIImpl.cpp 2005-02-23 00:09:50.256014648 +0100 @@ -1138,9 +1138,12 @@ mIsViewSource = vs; } + nsCOMPtr<nsIChannel> channel(do_QueryInterface(aRequest)); + PRBool hasChannel = !!channel; + mCurrentURI = aLocation; - if (isWyciwyg) { + if (isWyciwyg || hasChannel) { nsCOMPtr<nsIDOMWindow> windowForProgress; aWebProgress->GetDOMWindow(getter_AddRefs(windowForProgress));
Assignee | ||
Comment 5•19 years ago
|
||
Kaie: Welcome back, and thanks for the patch!!! A couple quick questions: * Why wouldn't yciwyg requests have a channel? * Why doesn't this bug show up on the trunk? (Is it that we sometimes don't have a channel on the branch, but always do on the trunk? What is your theory about that?)
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•19 years ago
|
||
If aRequest is null for a URI that should have a channel, then we have a pretty big problem. I think we need to ensure that it is non-null in those cases.
Assignee | ||
Comment 7•19 years ago
|
||
Ok, It appears that I simply failed to merge part of jst's patch for bug 277564: - FireOnLocationChange(this, nsnull, aURI); - - return NS_OK; + FireOnLocationChange(this, aRequest, aURI); } On the branches, nsnull is still passed :-( I'm updating my 1.0.1 firefox build to verify that fixing that fixes this bug as well as bug 283226, which is most likely a duplicate.
Assignee | ||
Comment 8•19 years ago
|
||
Ok, it turns out that I was right. I just failed to merge jst's patch properly. Here's the missing piece from nsDocShell::SetCurrentURI. I verified that this patch fixes this bug as well as the other.
Attachment #175241 -
Flags: superreview?(jst)
Attachment #175241 -
Flags: review?(jst)
Assignee | ||
Comment 9•19 years ago
|
||
*** Bug 283226 has been marked as a duplicate of this bug. ***
Comment 10•19 years ago
|
||
Comment on attachment 175241 [details] [diff] [review] v2 patch Yeah, that matches the trunk now. Good catch! r+sr=jst
Attachment #175241 -
Flags: superreview?(jst)
Attachment #175241 -
Flags: superreview+
Attachment #175241 -
Flags: review?(jst)
Attachment #175241 -
Flags: review+
Comment 11•19 years ago
|
||
Comment on attachment 175241 [details] [diff] [review] v2 patch a=asa for branch fixing.
Attachment #175241 -
Flags: approval-aviary1.0.1+
Comment 12•19 years ago
|
||
Comment on attachment 175241 [details] [diff] [review] v2 patch and 1.7.6 too.
Attachment #175241 -
Flags: approval1.7.6+
Assignee | ||
Comment 13•19 years ago
|
||
fixed1.7.6, fixed-aviary1.0.1
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Keywords: fixed-aviary1.0.1,
fixed1.7.6
Resolution: --- → FIXED
Comment 14•19 years ago
|
||
Verified Fixed with 2/23 builds for Aviary 1.0.1, Mozilla 1.7.6, and both Trunks. Going from mozilla.org to bugzilla only displays the security dialog once (with the security.warn_* prefs set to true). Going through a list of bugs no longer pops up the dialog each time you go to a new one.
Status: RESOLVED → VERIFIED
Reporter | ||
Updated•19 years ago
|
Group: security
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•