[Webkit-unassigned] [Bug 46335] Add EditingStyle

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Ryosuke Niwa <rniwa at webkit.org>  2010-10-22 14:37:23 PST ---
(From update of attachment 69542)
View in context: https://bugs.webkit.org/attachment.cgi?id=69542&action=review

> WebCore/editing/DeleteSelectionCommand.cpp:697
> +    prepareEditingStyleToApplyAt(m_typingStyle->style(), m_endingPosition);
> +    if (m_typingStyle->isEmpty())

oops, I should be calling prepareToApplyAt here.

>> WebCore/editing/DeleteSelectionCommand.cpp:720
>> +    document()->frame()->selection()->setTypingStyle(m_typingStyle ? m_typingStyle->style() : 0);
> 
> Any time we're reaching into the style like this I"m suspicious.
> Since we're changing selection too, shouldn't we add a setTypingStyle method which takes an editing styel and deprecate the other one?

Yeah, I'll add a version that takes EditingStyle.  I probably can't remove the old version because they're called in a bunch of other places.

>> WebCore/editing/SelectionController.h:243
>> +    m_typingStyle->clear();
> 
> Why not m_typingstyle = 0?

Because there are places we access m_typingStyle without a null check.

>> WebCore/editing/SelectionController.h:248
>> +    m_typingStyle->setStyle(style);
> 
> Seems odd to be reaching into the typeingstyle like this.  Will this go away?  Should this hav ea FIXME?

Yes, we should get rid of this version of setTypingStyle.  We should only have a version that takes EditingStyle.

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