[Bug 584632]

Mozilla-jorgk 584632 at bugs.launchpad.net
Tue Mar 24 18:23:27 UTC 2015


(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #88)
> Nit: you don't need to mention bug numbers in comments.  This information is
> available through hg/git blame.
There are many bug numbers in the code (including some that you entered for bug 596506), so this policy must have changed recently.

> Also, please wrap if bodies in braces.
Again, the policy seems to have changed here since there are many conditions of this style:
if (!range.content)
  return NS_ERROR_FAILURE;

> 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.
I disagree. I did what you suggest, but it didn't work. The
it->GetLine(thisLine, &firstFrame, &lineFrameCount, nonUsedRect);
ealier on gets the details for the previous line where the left arrow was pressed.
We then move on to the line before. The next
it->GetLine(currentLine, &currentFirstFrame, &lineFrameCount, usedRect);
is operating on the line we've just moved to, the one where we want to skip the brFrame.
The only thing which would be a bit cleaner is not to re-use the "it" variable but to declare another one.

I agree, I do need "atLineEdge".

-- 
You received this bug notification because you are a member of Mozilla
Bugs, which is subscribed to Mozilla.
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



More information about the Ubuntu-mozillateam-bugs mailing list