[webkit-reviews] review granted: [Bug 202904] Chromium test-case asserts with ASSERTION FAILED: propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect) : [Attachment 434825] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 202904: Chromium test-case asserts with ASSERTION FAILED:
propertyMissingOrEqualToNone(style, CSSPropertyWebkitTextDecorationsInEffect)
https://bugs.webkit.org/show_bug.cgi?id=202904

Attachment 434825: Patch

https://bugs.webkit.org/attachment.cgi?id=434825&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 434825
  --> https://bugs.webkit.org/attachment.cgi?id=434825
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434825&action=review

> Source/WebCore/editing/markup.cpp:387
> +    // With AnnotateForInterchange::Yes, wrappingStyleForSerialization
should have removed -webkit-text-decorations-in-effect
> +    ASSERT(!shouldAnnotate() || propertyMissingOrEqualToNone(style,
CSSPropertyWebkitTextDecorationsInEffect));

Changing the assertion is OK, but I think this is sort of missing the big
picture. Having a webpage specify "-webkit-text-decorations-in-effect" doesn’t
really make logical sense. It’s a property intended for internal use in the
engine. The key to this test case is that it specifies that property explicitly
in a style sheet.

Might be something to clean up here some day, but in the mean time loosening
this assertion is a fine thing to do so we don’t block fuzz testing. Changing
the assertion has no effect on WebKit behavior.