[Webkit-unassigned] [Bug 38564] Caret keeps blinking during forward-delete

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=38564


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #105823|review?                     |review+
               Flag|                            |




--- Comment #2 from Darin Adler <darin at apple.com>  2011-08-31 16:51:30 PST ---
(From update of attachment 105823)
View in context: https://bugs.webkit.org/attachment.cgi?id=105823&action=review

r=me, but I think this could be slightly improved coding-style-wise

I also would like to see us come up with a way to test.

> Source/WebCore/editing/FrameSelection.cpp:1604
> +    bool shouldStopBlinkingDueToTypingCommand = (m_frame && (lastEditCommand = m_frame->editor()->lastEditCommand())) ?
> +        lastEditCommand->shouldStopCaretBlinking() : false;

The last part of this should also be written using && rather than ? :

Also, this is hard to read with the assignment in the middle of it. I think it would be clearer like this:

    EditCommand* lastEditCommand = m_frame ? m_frame->editor()->lastEditCommand() : 0;
    bool shouldStopBlinkingDueToTypingCommand = lastEditCommand && lastEditCommand->shouldStopCaretBlinking();

> Source/WebCore/editing/TypingCommand.h:73
> +    bool shouldStopCaretBlinking() const { return true; }

This override could, and should, be private rather than public. It would be a programming mistake to call this on a TypingCommand*.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.