Last Comment Bug 658737 - Untrusted Connection page document doesn't clear busy state after loading
: Untrusted Connection page document doesn't clear busy state after loading
Status: VERIFIED FIXED
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla6
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-20 20:00 PDT by alexander :surkov
Modified: 2011-05-22 09:27 PDT (History)
2 users (show)
surkov.alexander: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (11.40 KB, patch)
2011-05-20 21:10 PDT, alexander :surkov
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review

Description alexander :surkov 2011-05-20 20:00:19 PDT
spun off 658185 comment #1.
Comment 1 alexander :surkov 2011-05-20 20:19:46 PDT
Boris, url of untrusted connection page doesn't container "neterror" string which we use to detect the error page (http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsCoreUtils.cpp#500). It contains "certerror" though. Should I test them both or is it ok to look for "error" string?
Comment 2 alexander :surkov 2011-05-20 20:21:04 PDT
btw, is there official way to get untrusted error page for mochitests? I could use url in bug 658185 but it may be temporary to rely on it in mochitest.
Comment 3 Boris Zbarsky [:bz] 2011-05-20 20:26:19 PDT
Hmm.  Yeah, you should check for about:certerror.
Comment 4 alexander :surkov 2011-05-20 21:10:07 PDT
Created attachment 534193 [details] [diff] [review]
patch
Comment 5 Trevor Saunders (:tbsaunde) 2011-05-20 22:26:54 PDT
Comment on attachment 534193 [details] [diff] [review]
patch


>@@ -331,30 +340,32 @@ private:
> 
> #define NS_LOG_ACCDOC_DOCINFO_BEGIN                                            \
>   printf("  {\n");
> #define NS_LOG_ACCDOC_DOCINFO_BODY(aDocument, aDocAcc)                         \

blank line?

>+      this.invoke = function loadUntrustedConnectionPageInvoker_invoke()
>+      {
>+        gTabBrowser.loadURI("https://bugzilla-mozilla-org.glb.mozilla.net/");
>+      }

I assume you know this URI will always produce a cert error?  I'm not sure if this is a remote site or if that URL points at some sort of test harness magic, but it seems like using a remote site would be undesirable.

I assume changing all the macros was to make things easier to debug?

r=me
Comment 6 Boris Zbarsky [:bz] 2011-05-20 22:32:27 PDT
Uh, yeah.  That's not harness magic.  That's an orange-waiting-to-happen.  Use https://nocert.example.com instead?  That one _is_ test harness magic.  Or you can use https://self-signed.example.com or  https://untrusted.example.com or  https://expired.example.com if you prefer, if you need a particular kind of about:certerror error page.
Comment 7 alexander :surkov 2011-05-20 22:34:38 PDT
(In reply to comment #5)
> Comment on attachment 534193 [details] [diff] [review] [review]
> patch
> 
> 
> >@@ -331,30 +340,32 @@ private:
> > 
> > #define NS_LOG_ACCDOC_DOCINFO_BEGIN                                            \
> >   printf("  {\n");
> > #define NS_LOG_ACCDOC_DOCINFO_BODY(aDocument, aDocAcc)                         \
> 
> blank line?

no I think, to keep them visually grouped, these are helper macroses

> 
> >+      this.invoke = function loadUntrustedConnectionPageInvoker_invoke()
> >+      {
> >+        gTabBrowser.loadURI("https://bugzilla-mozilla-org.glb.mozilla.net/");
> >+      }
> 
> I assume you know this URI will always produce a cert error?  I'm not sure
> if this is a remote site or if that URL points at some sort of test harness
> magic, but it seems like using a remote site would be undesirable.

I don't know better way, good thing is mozilla's address

> I assume changing all the macros was to make things easier to debug?

yes, these are all about crashes when macros is enabled.
Comment 8 alexander :surkov 2011-05-20 22:35:17 PDT
(In reply to comment #6)
> Uh, yeah.  That's not harness magic.  That's an orange-waiting-to-happen. 
> Use https://nocert.example.com instead?  That one _is_ test harness magic. 
> Or you can use https://self-signed.example.com or 
> https://untrusted.example.com or  https://expired.example.com if you prefer,
> if you need a particular kind of about:certerror error page.

thanks, Boris!
Comment 9 alexander :surkov 2011-05-20 22:36:53 PDT
though you know for all these pages I get "Firefox can't find the server at "
Comment 10 Boris Zbarsky [:bz] 2011-05-20 22:39:01 PDT
Uh... That's because they're test harness magic; see above.  Those URLs only do anything useful when running in the test harness.

I suggest generally reading http://mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt
Comment 11 Boris Zbarsky [:bz] 2011-05-20 22:40:13 PDT
And to be clear, linking to any external URLs in tests is not OK.  Ever.  In fact, the current plan is to run the tests on machines from which no outgoing connections are allowed at all.
Comment 12 alexander :surkov 2011-05-21 00:15:09 PDT
landed with mochitest fix - http://hg.mozilla.org/mozilla-central/rev/9132f5e5edf3
Comment 13 alexander :surkov 2011-05-21 00:23:29 PDT
(In reply to comment #11)
> And to be clear, linking to any external URLs in tests is not OK.  Ever.  In
> fact, the current plan is to run the tests on machines from which no
> outgoing connections are allowed at all.

That's right, I know. I asked the question in comment 2 but didn't get an answer, I thought you don't know and maybe there's not easy way to do that. I prefer to have a test rather than to not have it and if we get random orange then we can deal with it later. That's bad I know but allows me to get a fix with mochitest quickly.
Comment 14 Marco Zehe (:MarcoZ) on PTO until August 15 2011-05-22 09:27:46 PDT
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110522 Firefox/6.0a1

Note You need to log in before you can comment on or make changes to this bug.