test_nsIAccessibleDocument.html silently dies

VERIFIED FIXED in mozilla15

Status

()

Core
Disability Access APIs
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: ayg, Assigned: capella)

Tracking

(Blocks: 2 bugs)

Trunk
mozilla15
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

accessible/tests/mochitest/test_nsIAccessibleDocument.html seems to be running no tests, as indicated by <https://tbpl.mozilla.org/php/getParsedLog.php?id=10128146&tree=Try#error1>.  Serge says in bug 735805 comment 36:

> This a11y test is broken: getRootDirectory() is defined in
> chrome-harness.js, but including the latter just silently breaks somehow.
> Move this issue to a blocking bug.

For now I'll most likely have to add a todo(), because I can't figure out exactly how it's broken -- I have trouble getting it to work on localhost.
Summary: accessible/tests/mochitest/test_nsIAccessibleDocument.html is possibly broken → test_nsIAccessibleDocument.html silently fails
I got the test to work on localhost with Serge's help, and tracked down the basic problem -- including chrome-harness.js seems to break.  I commented it out for now, along with the check that relied on it, in a patch I'll attach to bug 735805.  I'm leaving this open as a followup to actually fix the test.  (Previously the test wasn't running any checks at all, but with the patch it's at least running some of them.)
No longer blocks: 735805
Depends on: 735805
Summary: test_nsIAccessibleDocument.html silently fails → test_nsIAccessibleDocument.html silently dies

Updated

5 years ago
Blocks: 417472
Whiteboard: [broken check disabled by bug 735805 patch 4.5]
(Assignee)

Comment 2

5 years ago
Created attachment 624467 [details] [diff] [review]
Patch (v1)

Small change ... asking surkov for feedback ...
Assignee: nobody → markcapella
Status: NEW → ASSIGNED
Attachment #624467 - Flags: feedback?(surkov.alexander)
Mark, do you have any idea about what goes wrong when using the chrome:// url in this test?

http://mxr.mozilla.org/mozilla-central/search?string=%2Fchrome-harness.js&case=1&find=%2Faccessible%2Ftests%2Fmochitest%2F
(Assignee)

Comment 4

5 years ago
Well, I played with it until I got the correct syntax. The below substitution works also...

Your original line was... src="chrome://mochikit/content/chrome-harness.js"/>
You could change it to... src="chrome://mochikit/content/chrome-harness.js"></script>
(In reply to Mark Capella [:capella] from comment #4)
> You could change it to...
> src="chrome://mochikit/content/chrome-harness.js"></script>

I would prefer that syntax over the '..' one in your patch ;-)

Yet, my question was really "is the breakage due to some code is that file?"...

Comment 6

5 years ago
Comment on attachment 624467 [details] [diff] [review]
Patch (v1)

Review of attachment 624467 [details] [diff] [review]:
-----------------------------------------------------------------

thank you for fixing this!

::: accessible/tests/mochitest/test_nsIAccessibleDocument.html
@@ +18,2 @@
>    <script type="application/javascript"
> +    src="../../chrome-harness.js"></script>

nit: please align it properly

@@ +39,5 @@
>          attributes = docAcc.attributes;
>          is(attributes.getStringProperty("tag"), "body",
>             "Wrong attribute on document!");
>  
> +        // Test for correct root directory.

if you want comments for this then it'd be nice to have
// Document URL

and

// Document title and mime type.

@@ +50,4 @@
>             "Wrong title for document!");
>          is(docAcc.mimeType, "text/html",
>             "Wrong mime type for document!");
> +

nit: whitespace on empty line
Attachment #624467 - Flags: feedback?(surkov.alexander) → review+

Comment 7

5 years ago
(In reply to Serge Gautherie (:sgautherie) from comment #5)

> Yet, my question was really "is the breakage due to some code is that
> file?"...

it seems so, HTML is not friendly to <script> close tag.
I feel extremely stupid about this.  Feeding it into validator.nu would have showed up the problem immediately -- self-closing tags are not allowed in .html files and don't do anything.  <foo /> is exactly the same as <foo>.  So it was being parsed as a <script> with both src="" and contents, and the contents were therefore being ignored.

Thanks for spotting this!  This is a motivation for fixing bug 736517, which would have caught the problem -- although apparently that's not easy.
(Assignee)

Comment 9

5 years ago
Created attachment 624706 [details] [diff] [review]
Patch (final)

This is the final version.

Comment 10

5 years ago
(In reply to Mark Capella [:capella] from comment #9)
> Created attachment 624706 [details] [diff] [review]
> Patch (final)
> 
> This is the final version.

cool, thank you!
Comment on attachment 624706 [details] [diff] [review]
Patch (final)

(In reply to Serge Gautherie (:sgautherie) from comment #3)
> http://mxr.mozilla.org/mozilla-central/search?string=%2Fchrome-harness.
> js&case=1&find=%2Faccessible%2Ftests%2Fmochitest%2F

Then: 1 XML with />, 2 HTML with </, and this broken HTML with />.


(In reply to Serge Gautherie (:sgautherie) from comment #5)
> (In reply to Mark Capella [:capella] from comment #4)
> > You could change it to...
> > src="chrome://mochikit/content/chrome-harness.js"></script>
> 
> I would prefer that syntax over the '..' one in your patch ;-)

Please, do use </script> as in the 2 other HTML files.
Attachment #624706 - Flags: feedback-
Blocks: 736517
(Assignee)

Comment 12

5 years ago
Serge, is there something you'd like to suggest we change in the final patch? I don't see your concern at this stage.
I meant "use your comment 4 suggestion": no need to add a new/third (less good) syntax.
(Assignee)

Comment 14

5 years ago
Without objections.

TRY push ... https://tbpl.mozilla.org/?tree=Try&rev=a77b0a028414
(Assignee)

Comment 15

5 years ago
Inbound status:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=a56ebbf0f293
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/a56ebbf0f293
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1337377564.1337381641.22500.gz&fulltext=1
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/05/18 14:46:04
{
16725 INFO TEST-PASS | chrome://mochitests/content/a11y/accessible/test_nsIAccessibleDocument.html | Wrong URL for document! - chrome://mochitests/content/a11y/accessible/test_nsIAccessibleDocument.html should equal chrome://mochitests/content/a11y/accessible/test_nsIAccessibleDocument.html
16726 INFO TEST-PASS | chrome://mochitests/content/a11y/accessible/test_nsIAccessibleDocument.html | Wrong title for document! - nsIAccessibleDocument chrome tests should equal nsIAccessibleDocument chrome tests
}

V.Fixed
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Whiteboard: [broken check disabled by bug 735805 patch 4.5]
You need to log in before you can comment on or make changes to this bug.