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, ¤tBlockFrame); > + nsAutoLineIterator it = currentBlockFrame->GetLineIterator(); > + it->GetLine(currentLine, ¤tFirstFrame, &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 -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp