Bug 120114 – [css3-text] Add editing test for CSS3 Text Decoration properties

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Bruno Abinader]

This bug intends to add an editing layout test involving CSS3 Text Decoration properties, as requested in webkit-dev:
https://lists.webkit.org/pipermail/webkit-dev/2013-August/025304.html

[Bruno Abinader]

Created attachment 209269 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 209269 [details]
Patch

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

> LayoutTests/ChangeLog:10
> +        Adds editing layout test to verify that CSS3 Text Decoration properties
> +        are being properly propagated. Skipping Mac, Qt and Win as they do not
> +        enable CSS3_TEXT feature flag by default.

We also need tests for copying & pasting a descendent of a node with text-decoration properties set since we ought to preserve the computed style in that case.
In addition, we need tests for toggling underline and strike through with these styles present as well as tests to merge paragraphs (i.e. deleting line breaks) with and without these styles.
We should also test what happens when the user types before/after or deletes text with these text decorations.

> LayoutTests/editing/pasteboard/insert-text-decoration.html:16
> +            document.querySelector('div[contenteditable]').focus();

Why don't we just do document.getElementById('container').focus() instead.
Btw, I hate indentations in this file.

[Bruno Abinader]

(In reply to comment #2)
> (From update of attachment 209269 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=209269&action=review
> 
> > LayoutTests/ChangeLog:10
> > +        Adds editing layout test to verify that CSS3 Text Decoration properties
> > +        are being properly propagated. Skipping Mac, Qt and Win as they do not
> > +        enable CSS3_TEXT feature flag by default.
> 
> We also need tests for copying & pasting a descendent of a node with text-decoration properties set since we ought to preserve the computed style in that case.
> In addition, we need tests for toggling underline and strike through with these styles present as well as tests to merge paragraphs (i.e. deleting line breaks) with and without these styles.
> We should also test what happens when the user types before/after or deletes text with these text decorations.

Ack. Regarding this, while implementing the toggling underline/strikethrough editing test I noticed an issue with mixed usage of text-decoration/-webkit-text-decoration-line. Currently, these two properties shares the same data internally, however, this confuses editing code. I'll create a new bug for this to be resolved separately.

> 
> > LayoutTests/editing/pasteboard/insert-text-decoration.html:16
> > +            document.querySelector('div[contenteditable]').focus();
> 
> Why don't we just do document.getElementById('container').focus() instead.
> Btw, I hate indentations in this file.

No problem. I was following Julien's recommendation on having indented code, but I can switch back to the  indentation style from other WebKit layout tests.

[Darin Adler]

Comment on attachment 209269 [details]
Patch

Seems good to land some new tests. But this patch is really old, I doubt it will apply.

[Bruno Abinader]

(In reply to comment #4)
> (From update of attachment 209269 [details])
> Seems good to land some new tests. But this patch is really old, I doubt it will apply.

Hi Darin, thanks for the update :) Sure, I can rebase them if necessary.

[Bruno Abinader]

Created attachment 234853 [details]
Rebased patch for landing

[Build Bot]

Comment on attachment 234853 [details]
Rebased patch for landing

Attachment 234853 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6563447495983104

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html

[Build Bot]

Created attachment 234858 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Bruno Abinader]

Comment on attachment 234853 [details]
Rebased patch for landing

The failure in mac-wk2 is not related with this change.

[WebKit Commit Bot]

Comment on attachment 234853 [details]
Rebased patch for landing

Clearing flags on attachment: 234853

Committed r171067: <http://trac.webkit.org/changeset/171067>