desktop-packages team mailing list archive

[Ehsan-mozilla <584632@xxxxxxxxxxxxxxxxxx>]

Comment on attachment 8582360
Patch to fix all three problems: Click, "end" key, left arrow from start of previous line

Review of attachment 8582360:
-----------------------------------------------------------------

Some feedback on your patch so far.

::: layout/generic/nsFrame.cpp
@@ +3555,3 @@
>    for (int32_t n = aLine->GetChildCount(); n;
>         --n, frame = frame->GetNextSibling()) {
> +    // Bug 756984: Skip brFrames. Can only skip if the line contains at least one selectable and non-empty frame before

Nit: you don't need to mention bug numbers in comments.  This
information is available through hg/git blame.

@@ +3555,4 @@
>    for (int32_t n = aLine->GetChildCount(); n;
>         --n, frame = frame->GetNextSibling()) {
> +    // Bug 756984: Skip brFrames. Can only skip if the line contains at least one selectable and non-empty frame before
> +    if (!SelfIsSelectable(frame, aFlags) || frame->IsEmpty() || (canSkipBr && frame->GetType() == nsGkAtoms::brFrame))

Nit: please wrap lines at 80 characters.  For example, according to our
coding style, this condition should be written as:

  if (!SelfIsSelectable(frame, aFlags) || frame->IsEmpty() ||
      (canSkipBr && frame->GetType() == nsGkAtoms::brFrame))

@@ +6801,5 @@
>          for (int32_t count = lineFrameCount; count;
>               --count, frame = frame->GetNextSibling()) {
>            if (!frame->IsGeneratedContentFrame()) {
> +            // Bug 756984: When jumping to the end of the line with the "end" key, skip over brFrames
> +            if (endOfLine && lineFrameCount > 1 && frame->GetType() == nsGkAtoms::brFrame) continue;

Nit: again, please adjust the whitespace here.  Also, please wrap if
bodies in braces.

@@ +7090,5 @@
> +      nsIFrame *currentBlockFrame, *currentFirstFrame;
> +      nsRect usedRect;
> +      int32_t currentLine = nsFrame::GetLineNumber(traversedFrame, aScrollViewStop, &currentBlockFrame);
> +      nsAutoLineIterator it = currentBlockFrame->GetLineIterator();
> +      it->GetLine(currentLine, &currentFirstFrame, &lineFrameCount, usedRect);

There is already code which gets the line frame count earlier in this
function.  Please refactor it out of the else block it currently lives
in and just use lineFrameCount from that code here.  That already takes
care of checking the return value of GetLine for errors too.  Also, I
think you want to check atLineEdge instead of aJumpLines here.

-- 
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