Closed Bug 658737 Opened 9 years ago Closed 9 years ago

Untrusted Connection page document doesn't clear busy state after loading

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla6

People

(Reporter: surkov, Assigned: surkov)

Details

(Keywords: access)

Attachments

(1 file)

spun off 658185 comment #1.
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?
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.
Hmm.  Yeah, you should check for about:certerror.
Attached patch patchSplinter Review
Attachment #534193 - Flags: review?(trev.saunders)
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
Attachment #534193 - Flags: review+
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.
(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.
(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!
though you know for all these pages I get "Firefox can't find the server at "
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
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.
landed with mochitest fix - http://hg.mozilla.org/mozilla-central/rev/9132f5e5edf3
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
(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.
Attachment #534193 - Flags: review?(trev.saunders)
Verified fixed in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:6.0a1) Gecko/20110522 Firefox/6.0a1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.