Bug 120114

Created attachment 209269 [details] Patch

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.

(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.

Comment on attachment 209269 [details] Patch Seems good to land some new tests. But this patch is really old, I doubt it will apply.

(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.

Created attachment 234853 [details] Rebased patch for landing

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

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

Comment on attachment 234853 [details] Rebased patch for landing The failure in mac-wk2 is not related with this change.

Comment on attachment 234853 [details] Rebased patch for landing Clearing flags on attachment: 234853 Committed r171067: <http://trac.webkit.org/changeset/171067>