Comment on attachment 8583354 Test case (revised) Review of attachment 8583354: -----------------------------------------------------------------
Looks like a great start! Did you intend to test the rest of the cases in a separate patch? ::: layout/generic/test/test_bug756984.html @@ +13,5 @@ > +<body> > +<a target="_blank" > href="https://bugzilla.mozilla.org/show_bug.cgi?id=756984";>Mozilla Bug > 756984</a> > +<p id="display"></p> > +<div id="content" style="display: none"> > +</div> Since you mentioned you're new to writing tests, I'll add some bits about how this stuff works here. I hope that helps! The above is basically boilerplate that we include in most mochitests. The only parts that are really required are the two script elements that load the SimpleTest and event synthesis APIs. @@ +25,5 @@ > + > + /** Test for Bug 756984 **/ > + /** We test that clicking beyond the end of a line terminated with <br> > selects the preceding text, if any **/ > + > + SimpleTest.waitForExplicitFinish(); This is used to tell the test harness to not stop the test once the page load finishes (which is the default.) This is required for all tests that can run past that point. For example, this test is asynchronous because of the waitForFocus call below. @@ +27,5 @@ > + /** We test that clicking beyond the end of a line terminated with <br> > selects the preceding text, if any **/ > + > + SimpleTest.waitForExplicitFinish(); > + > + SimpleTest.waitForFocus(function() { This is required to ensure that the test window is raised to the top on the desktop, which is needed to ensure proper event delivery. @@ +35,5 @@ > + var sel = window.getSelection(); > + var f = document.getElementById("div1"); > + theDiv.focus(); > + sel.collapse(theDiv, 0); > + synthesizeMouse(theDiv, 100, 2, {type: "mousedown"}, f.contentWindow); The last argument of synthesizeMouse is the window object corresponding to the element that should receive the event. If omitted, the default is the current window object. Now, f is an HTMLDivElement, which doesn't have a contentWindow property, so f.contentWindow ends up as undefined. The reason this works is that from the perspective of the callee function, omitted arguments are undefined, so that's what this function checks for: <https://dxr.mozilla.org/mozilla- central/source/testing/mochitest/tests/SimpleTest/EventUtils.js#814>. So please just remove this last argument here and elsewhere in the test. @@ +36,5 @@ > + var f = document.getElementById("div1"); > + theDiv.focus(); > + sel.collapse(theDiv, 0); > + synthesizeMouse(theDiv, 100, 2, {type: "mousedown"}, f.contentWindow); > + synthesizeMouse(theDiv, 100, 2, {type: "mouseup"}, f.contentWindow); Nit: You can just issue one synthesizeMouse as {type: "click"} and it will dispatch both of these events internally. @@ +37,5 @@ > + theDiv.focus(); > + sel.collapse(theDiv, 0); > + synthesizeMouse(theDiv, 100, 2, {type: "mousedown"}, f.contentWindow); > + synthesizeMouse(theDiv, 100, 2, {type: "mouseup"}, f.contentWindow); > + var selObj = window.getSelection(); Nit: You already have |sel| above, no need to get it again, please remove this variable. @@ +49,5 @@ > + f = document.getElementById("div2"); > + theDiv.focus(); > + sel.collapse(theDiv, 0); > + synthesizeMouse(theDiv, 100, 2, {type: "mousedown"}, f.contentWindow); > + synthesizeMouse(theDiv, 100, 2, {type: "mouseup"}, f.contentWindow); Hmm, doesn't this also click to the right of the first line? @@ +53,5 @@ > + synthesizeMouse(theDiv, 100, 2, {type: "mouseup"}, f.contentWindow); > + selObj = window.getSelection(); > + selRange = selObj.getRangeAt(0); > + is(selRange.endContainer.nodeName, "DIV", "selection should be in > DIV"); > + is(selRange.endOffset, 0, "offset should be 0"); Not sure if I understand why this result would be desirable. I think that because you're clicking at the end of the first line, this is Gecko putting the selection at the end of the first line, which is on the first br element. Am I missing something? @@ +55,5 @@ > + selRange = selObj.getRangeAt(0); > + is(selRange.endContainer.nodeName, "DIV", "selection should be in > DIV"); > + is(selRange.endOffset, 0, "offset should be 0"); > + > + SimpleTest.finish(); This tells the test framework that the asynchronous test has been finished. This should be the last thing that the test does. If you have called waitForExplicitFinish in your test, it is an error to forget to call SimpleTest.finish(). -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/584632 Title: composer changes font mid email To manage notifications about this bug go to: https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
