[Webkit-unassigned] [Bug 55349] WebKit does not merge text decorations in the typing style and the selected element properly

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

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




--- Comment #2 from Darin Adler <darin at apple.com>  2011-03-01 10:26:42 PST ---
(From update of attachment 84201)
View in context: https://bugs.webkit.org/attachment.cgi?id=84201&action=review

> Source/WebCore/editing/ApplyStyleCommand.cpp:1284
> +    if (node->isHTMLElement() && static_cast<HTMLElement*>(node)->inlineStyleDecl()) {
> +        newInlineStyle = style->copy();
> +        newInlineStyle->mergeInlineStyleOfElement(static_cast<HTMLElement*>(node));
>      }

Why is this code specific to HTMLElement? Can’t this be done on a custom element that is technically not an HTML element? This code would treat <subsection>, for example, differently from <section>. I am not sure I understand why we have so much code specific to HTMLElement.

> Source/WebCore/editing/EditingStyle.cpp:714
> +        bool shouldAddTextDecorations = false;
> +
> +        RefPtr<CSSValue> value;

I think it would be clearer to combine these two local variables. The fact that “value” is specifically a text decorations value is unclear given the current name, and I think that the value being a null pointer would be just as good an indicator as the boolean if it had a good name.

Or maybe not.

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