[Webkit-unassigned] [Bug 179431] Inserting an image, selecting, underlining, and then deleting leaves the typing style with both "-webkit-text-decorations-in-effect" and "text-decoration"

[bugzilla-daemon at webkit.org]

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

--- Comment #2 from Wenson Hsieh <wenson_hsieh at apple.com> ---
The debug assertion being hit is in reconcileTextDecorationProperties:

    ASSERT(!textDecorationsInEffect || !textDecoration);

so we expect that "-webkit-text-decorations-in-effect" does not appear in the EditingStyle simultaneously as "text-decoration". This was added way back in <https://trac.webkit.org/r46286>. This pathological EditingStyle is computed in DeleteSelectionCommand::saveTypingStyleState, within this line:

    m_typingStyle = EditingStyle::create(m_selectionToDelete.start(), EditingStyle::EditingPropertiesInEffect);

in particular, by this chunk of code, in EditingStyle::init:

    if (propertiesToInclude == EditingPropertiesInEffect) {
        if (RefPtr<CSSValue> value = backgroundColorInEffect(node))
            m_mutableStyle->setProperty(CSSPropertyBackgroundColor, value->cssText());
        if (RefPtr<CSSValue> value = computedStyleAtPosition.propertyValue(CSSPropertyWebkitTextDecorationsInEffect))
            m_mutableStyle->setProperty(CSSPropertyTextDecoration, value->cssText());
    }

which adds the "text-decoration" property to EditingStyle if "-webkit-text-decorations-in-effect" is present. This logic was added in <https://trac.webkit.org/r98794>. So the situation looks like this right now:

1. DeleteSelectionCommand::saveTypingStyleState computes a typing style
2. In doing so, it calls into EditingStyle::init, which observes that "-webkit-text-decorations-in-effect" is present and tacks on a "text-decoration" with an identical CSS value to the EditingStyle's mutable style properties.
3. DeleteSelectionCommand::calculateTypingStyleAfterDelete sets the current selection's typing style to the above typing style
4. Later on, when we try to insert text, we compute the StyleChange using the above typing style, which calls into reconcileTextDecorationProperties.
5. reconcileTextDecorationProperties debug asserts that "-webkit-text-decorations-in-effect" and "text-decoration" don't coexist on the EditingStyle's (i.e. the typing style's) mutable properties.
6. We crash on debug, because (2) made sure that both properties are present!

At a quick glance, there are a few ways to address this:

    (a) Remove the debug assert in reconcileTextDecorationProperties, and have it instead remove one of the text decoration properties if both of them are present. In other words, don't have reconcileTextDecorationProperties crash if both properties exist; instead, make reconcileTextDecorationProperties actually reconcile these two properties.

-or-

    (b) Tweak the debug assert so it only fires if the values of "-webkit-text-decorations-in-effect" and "text-decoration" are different. Is there actually any harm in having both properties on the EditingStyle, if they agree on value?

-or-

    (c) Change EditingStyle::init so that when it doesn't result in having both "-webkit-text-decorations-in-effect" and "text-decoration", by removing "-webkit-text-decorations-in-effect" from the mutable style property after adding "text-decoration". FWIW, I ran the tests in editing/ against iOS simulator after making this change, and nothing failed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171108/47c3ffdf/attachment.html>