Bug 12562 – Fix test cases that are now spewing console errors that they are not testing for

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Sam Weinig]

After the recent change to the DRT to dump messages to the console, it became apparent that some tests have errors in them that they are not testing for.  Forthcoming patch fixes a bunch of these.

[Sam Weinig]

Created attachment 12903 [details]
patch

[Adam Roben (:aroben)]

Comment on attachment 12903 [details]
patch

   This is an awesome patch. Some of the expected results seem a bit questionable, though.

-SUCCESS - Didn't crash
+SUCCESS

-    <div>SUCCESS - Didn't crash</div>
+    <div id="result">SUCCESS - Didn't crash</div>

   The two above changes don't seem to match up. Why is " - Didn't crash" not appearing in the expected results?

+caret: position 1 of child 0 {INPUT} of child 0 {DIV} of child 0 {BODY} of child 0 {HTML} of document

   If the caret is inside the <input>, won't it have a focus ring (which is what this test says shouldn't happen)?

+try { [2, 5, 9].forEach(printEltAndException); }
+catch (e) { print(e); }

   Can you add newlines here as if it were an if-else block?

+try { [2, 5, 8, 1, 4].some(isBigEnoughAndException); }
+catch (e) { print(e); }
+
+try { [12, 5, 8, 1, 44].some(isBigEnoughAndException); }
+catch (e) { print(e); }

   Ditto.

-caret: position 9 of child 0 {#text} of child 7 {DIV} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+selection start: position 9 of child 0 {#text} of child 3 {DIV} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+selection end:   position 0 of child 5 {DIV} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document

   Why are we ending up with a selection now?

[Sam Weinig]

(In reply to comment #2)
> (From update of attachment 12903 [details] [edit])
>    This is an awesome patch. Some of the expected results seem a bit
> questionable, though.
> 
> -SUCCESS - Didn't crash
> +SUCCESS
> 
> -    <div>SUCCESS - Didn't crash</div>
> +    <div id="result">SUCCESS - Didn't crash</div>
> 
>    The two above changes don't seem to match up. Why is " - Didn't crash" not
> appearing in the expected results?

It now displays only 'SUCCESS' because the way the patch was set up was to  call the js which called         document.getElementById('result').innerHTML = 'SUCCESS'; 
Before, the id was not set on the div, so this call could not be completed.  I could get rid of this all together and the output would not change.

> +caret: position 1 of child 0 {INPUT} of child 0 {DIV} of child 0 {BODY} of
> child 0 {HTML} of document
> 
>    If the caret is inside the <input>, won't it have a focus ring (which is
> what this test says shouldn't happen)?

I'm not exactly sure what is happening here (I need to investigate it further) but this is not clicking inside the <input>.  If it was, with a eventSender.mouseMoveTo(100, 45) like its sister test input-text-click-inside.html, the png would show focus ring.  As I said I'm not exactly sure what the caret: ... syntax means in this case.

> +try { [2, 5, 9].forEach(printEltAndException); }
> +catch (e) { print(e); }
> 
>    Can you add newlines here as if it were an if-else block?
> 
> +try { [2, 5, 8, 1, 4].some(isBigEnoughAndException); }
> +catch (e) { print(e); }
> +
> +try { [12, 5, 8, 1, 44].some(isBigEnoughAndException); }
> +catch (e) { print(e); }
> 
>    Ditto.

I did it in this style to be consistent with the rest of the test.  I will change it.

> -caret: position 9 of child 0 {#text} of child 7 {DIV} of child 7 {DIV} of
> child 1 {BODY} of child 0 {HTML} of document
> +selection start: position 9 of child 0 {#text} of child 3 {DIV} of child 7
> {DIV} of child 1 {BODY} of child 0 {HTML} of document
> +selection end:   position 0 of child 5 {DIV} of child 7 {DIV} of child 1
> {BODY} of child 0 {HTML} of document
> 
>    Why are we ending up with a selection now?
> 

We are selecting the third table element.  At first I thought that was correct, but now I can't see why.  Will look into it further as well.

[Sam Weinig]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 12903 [details] [edit] [edit])

> > +caret: position 1 of child 0 {INPUT} of child 0 {DIV} of child 0 {BODY} of
> > child 0 {HTML} of document
> > 
> >    If the caret is inside the <input>, won't it have a focus ring (which is
> > what this test says shouldn't happen)?
> 
> I'm not exactly sure what is happening here (I need to investigate it further)
> but this is not clicking inside the <input>.  If it was, with a
> eventSender.mouseMoveTo(100, 45) like its sister test
> input-text-click-inside.html, the png would show focus ring.  As I said I'm not
> exactly sure what the caret: ... syntax means in this case.

I talked to mitz on IRC about this and he says this is the correct behavior.  This indicates that the caret is not inside the input, which the png also shows.

> > -caret: position 9 of child 0 {#text} of child 7 {DIV} of child 7 {DIV} of
> > child 1 {BODY} of child 0 {HTML} of document
> > +selection start: position 9 of child 0 {#text} of child 3 {DIV} of child 7
> > {DIV} of child 1 {BODY} of child 0 {HTML} of document
> > +selection end:   position 0 of child 5 {DIV} of child 7 {DIV} of child 1
> > {BODY} of child 0 {HTML} of document
> > 
> >    Why are we ending up with a selection now?
> > 
> 
> We are selecting the third table element.  At first I thought that was correct,
> but now I can't see why.  Will look into it further as well.
> 

I am changing this so that we no longer have a selection by increasing the leapForward time to 1000ms.

New patch forthcoming.

[Sam Weinig]

Created attachment 12918 [details]
updated patch

[Darin Adler]

Comment on attachment 12918 [details]
updated patch

The console error part of this patch looks great. r=me on that.

But the changes inside ksvg2 seem unrelated and probably shouldn't be in this patch.

[Sam Weinig]

Created attachment 12926 [details]
updated patch

Oops!  I forgot those where in my tree.  Removed.

[Darin Adler]

Comment on attachment 12926 [details]
updated patch

r=me

[Sam Weinig]

Landed in r19397.