← Back to team overview

desktop-packages team mailing list archive

  1. desktop-packages team
  2. Mailing list archive
  3. Message #109393

[Bug 584632]

  Thread Previous • Date Previous • • Thread Next 
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 Desktop
Packages, which is subscribed to thunderbird in Ubuntu.
https://bugs.launchpad.net/bugs/584632

Title:
  composer changes font mid email

Status in Mozilla Thunderbird Mail and News:
  Confirmed
Status in thunderbird package in Ubuntu:
  Confirmed

Bug description:
  Binary package hint: thunderbird

  As I'm typing my emails in Thunderbird, I can see what appears to be a
  font size change on screen, normally in the second line of text. The
  second line appear smaller than the first. It's barely perceptible, so
  half them time I think I am imagining it.

  Well, I've started Bccing to myself to check, and the emails I am
  receiving from myself are not only a different size, they're also a
  different font. Composer starts in some default serif, and by the
  second line is sans. I'd bee glad to email someone viz thunderbird,
  and also send along a screenshot of how it looks while I am typing.

  Thanks.

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/584632/+subscriptions
  Thread Previous • Date Previous • • Thread Next