[Webkit-unassigned] [Bug 118185] Gmail reply email - Bold and Italic style get stuck

[bugzilla-daemon at webkit.org]

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





--- Comment #33 from Ryosuke Niwa <rniwa at webkit.org>  2013-07-30 02:07:30 PST ---
(In reply to comment #32)
> (In reply to comment #31)
> > (From update of attachment 207698 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=207698&action=review
> > 
> > > Source/WebCore/editing/EditingStyle.cpp:691
> > >          TriState nodeState = triStateOfStyle(&computedStyle, node->isTextNode() ? EditingStyle::DoNotIgnoreTextOnlyProperties : EditingStyle::IgnoreTextOnlyProperties);
> > >          if (node == selection.start().deprecatedNode())
> > >              state = nodeState;
> > > -        else if (state != nodeState && node->isTextNode()) {
> > > +        else if (state != nodeState && node->isTextNode() && node->renderer() && node->rendererIsEditable()) {
> > 
> > Come to think of it, this code change is probably incorrect. We shouldn't be updating state when node doesn't have a renderer or it's not editable
> > although I can't think of a test case in which start().deprecatedNode() will be either invisible or not editable.
> > 
> > We should nonetheless make this code more robust by skipping any non-editable node or any node without renderer.
> > Note that we need to create a local boolean to replace "node == selection.start().deprecatedNode()" in that case
> > since we need to skip until we see the first editable node with renderer.
> 
> Thanks for the review. I am bit confused here. To make sure that we update the state only when start().deprecatedNode() has renderer and editable wouldn't the below check be sufficient 
> 
> if (node == selection.start().deprecatedNode() && node->renderer() && node->rendererIsEditable())
> 
> Why do we need the local boolean?

No, because state must be updated when we computed the style for the first editable node with renderer. If start().deprecatedNode() didn't have renderer, then "node == selection.start().deprecatedNode()" will never be true for subsequent nodes and never update state to true state.

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