posting from Asa's blog: 1. If I enter the URL: https://agia.fsf.org/order/ I get the padlock in the bottom and the address bar is yellow. 2. Now if I click on one of the #refs, eg the link "Software", I lose the padlock and the yellow address bar. 3. Pasting the full url https://agia.fsf.org/order/#software gives the secure feedback, but clicking another #ref loses it again. This is with 1.0.1 en-us Linux ------------------------------------ I was able to reproduce this somewhat using a welsh (cy-GB, have been doing l10n testing, I hope this doesn't matter) 20050223-1.0.1 build. (I can recheck later with en-US.) Anyhow, I get the same misbehavior at step 2 --but strangely, step 3 for me doesn't restore the locked padlock or yellow background color in the address bar. I tested this using a ffox 1.0 build (mac again), and this was not a problem.

yikes... this sounds pretty bad.

This is public (http://weblogs.mozillazine.org/asa/archives/007626.html) and not a security hole. Why is it marked as security-sensitive?

Are people seeing this on the trunk, or is it only affecting Firefox 1.0.1?

I cannot reproduce this using 2005021409-trunk ffox bits (linux fc3). I'll grab a more recent build...

hrm, I do see this using 2005022511-trunk ffox bits. Jay had email some https / location bar / security / padlock bugs with checkins which might be suspect: https://bugzilla.mozilla.org/show_bug.cgi?id=258048 https://bugzilla.mozilla.org/show_bug.cgi?id=268483 https://bugzilla.mozilla.org/show_bug.cgi?id=277564 https://bugzilla.mozilla.org/show_bug.cgi?id=283201

Testing Windows, this regressed on the 1.0.1 branch between 2005-02-14-06-aviary1.0.1 and 2005-02-15-06-aviary1.0.1. Sarah, please check the trunk build from the 15th.

using ffox trunk builds on linux fc3, this regressed between: * 2005021510-trunk (works) * 2005021609-trunk (broken) another observation: after clicking a relative link (then noticing the locked UI go away), reloading (ctrl+R) the page restores the locked UI.

Thanks for narrowing the regression window. It looks like the patch for bug 258048 is the likely cause of this bug. Investigating...

OK, the problem here is quite simple. We are getting a null request in the OnLocationChange event generated for navigations to https://blah.com/#ref I could attempt to fix this bug by skipping the lock icon updating logic when we have a null request (or perhaps when we have a null channel). What do people think of that solution? It may also be wrong that we are sending an OnLocationChange in this case (since there is no corresponding request), but then again... I think that might be necessary to ensure that the URL bar is updated properly. I wonder what other cases may result in a null request. Is it always valid to ignore those and assume that they have the same security level?

Comment on attachment 175629 [details] [diff] [review] v1 patch This seems reasonable to me. r=jst

Clearing the security flag for reasons Jesse noted in comment 2

Comment on attachment 175629 [details] [diff] [review] v1 patch sr- This regresses recently fixed bug 258048 and bug 277564 As far as I can tell aRequest is *always* null when passed to OnLocationChange.

-> PSM

dveditz: no! if you recall, i fixed the regression where aRequest was always null. that regression was caused by a bad merge to nsDocShell. i tested this very carefully, and aRequest is only null in special cases such as this one where there is not a network request. otherwise, due to johnny's patch, there is a non-null network request. please re-consider the patch.

Comment on attachment 175629 [details] [diff] [review] v1 patch With this patch applied I could not reproduce bug 258048. Did you find that you could? restoring r=jst

Isn't aRequest null on tab switches? Does the lock icon update properly on tab switch with this change?

Comment on attachment 175629 [details] [diff] [review] v1 patch ah, *docshell*. I made sure to cvs up in security before testing your patch, but forgot the late-breaking docshell change. :-( Sorry about that, it of course works as advertised (*phew*, I was wondering how in the world the other patches worked on a null request). sr=dveditz

I think this should block 1.7.6 just to get it right, but since it shows a secure site as non-secure rather than the other way around it's not a security hole that requires a firefox respin.

Comment on attachment 175629 [details] [diff] [review] v1 patch requesting 1.7.6 approval, and 1.0.1 branch for the heck of it just in case there's a 1.0.2

(In reply to comment #17) > Isn't aRequest null on tab switches? Does the lock icon update properly on tab > switch with this change? No, nsSecureBrowserUI::OnLocationChange is not called when tabs are switched. If that is using nsIWebProgressListener, then it must depend on something special being done by tabbrowser.xml for nsIWebProgressListener's attached directly to it. I suspect that the tabbrowser stores the state of each browser, and notifies its listeners when things change. The nsSecureBrowserUI is attached to each browser.

I think it would be wise to consider respinning for this fix since it can give users false confidence in the browser. It may for example make some ecommerce site appear insecure, which might discourage customers, etc. That seems like something worth fixing. Wasn't there a respin planned for the installer problem?

(In reply to comment #22) > Wasn't there a respin planned for the installer problem? The respin was for 1.0.1 and included no app-level changes, only installer changes. Happened last night, files have been QA'd, signed, and are ready to be pushed onto the site.

ok, fair enough. i don't know how best to handle this.

fixed-on-trunk

The tabbrowser code is at http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/tabbrowser.xml#572 Looks like the security UI is just not a listener on the <tabbrowser>, if this code doesn't break us....

Comment on attachment 175629 [details] [diff] [review] v1 patch 1.0.1 has shipped; requesting approval for aviary 1.0.2

Comment on attachment 175629 [details] [diff] [review] v1 patch approved.

Can someone land this fix on the appropriate branches?

yes, i will do so shortly.

fixed-aviary1.0.1, fixed1.7.6

*** Bug 285241 has been marked as a duplicate of this bug. ***

(In reply to comment #31) > fixed-aviary1.0.1, fixed1.7.6 Hmm fixed-aviary1.0.2 I guess.

tested with 2005031011-1.7 mozilla bits on linux fc3: looks good, the locked padlock in the statusbar doesn't disappear when I click a relative anchor in the secure page.

mistakenly removed fixed1.7.6 --pardon the bugspam. set your filter/quicksearch to "ZippidityDooDahHey" to catch these for easy removal/etc/

also verified fixed in - firefox 1.0.2 branch 20050314 build on Mac OS X 10.3.8 - firefox trunk 20050314 builds on Mac and Win XPsp2.