Takes an example from 12319. Copying some text from TextEdit in the rich format mode results in verbose inline styles such as: <p style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font-family: Helvetica; font-size: 12px; ">this is line one</p> It could be <p style="margin: 0px; font-family: Helvetica; font-size: 12px; ">this is line one</p> instead.
Created attachment 132987 [details] work in progress
Created attachment 133169 [details] work in progress 2 There appear to be some regressions in copy & paste.
Created attachment 133205 [details] work in progress 3 I just need a test now.
Attachment 133205 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/editing/pasteboard/paste-and-s..." exit_code: 1 Source/WebCore/css/StylePropertySet.h:103: Missing spaces around = [whitespace/operators] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
It appears that we can do this for all cases. e.g. Firefox uses shorthand notations for cssText as well.
Created attachment 133337 [details] work in progress 4 Enable shorthands for all cases.
Comment on attachment 133337 [details] work in progress 4 Attachment 133337 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12116275 New failing tests: editing/pasteboard/paste-and-sanitize.html inspector/styles/styles-update-from-js.html fast/css/remove-shorthand.html editing/style/non-inheritable-styles.html fast/css/uri-token-parsing.html printing/page-rule-css-text.html
Created attachment 133453 [details] work in progress 5
Created attachment 133590 [details] Patch
Comment on attachment 133590 [details] Patch Attachment 133590 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12117885
Comment on attachment 133590 [details] Patch Attachment 133590 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12117887
Comment on attachment 133590 [details] Patch Attachment 133590 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12122879
Comment on attachment 133590 [details] Patch Attachment 133590 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12123847
Comment on attachment 133590 [details] Patch Attachment 133590 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12116920
Created attachment 133596 [details] Added the missing BitVector change
Comment on attachment 133596 [details] Added the missing BitVector change Attachment 133596 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12117902
Comment on attachment 133596 [details] Added the missing BitVector change Attachment 133596 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/12119007
Comment on attachment 133596 [details] Added the missing BitVector change Attachment 133596 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/12097058
Created attachment 133601 [details] Another build fix
Comment on attachment 133601 [details] Another build fix Attachment 133601 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12122905
Comment on attachment 133601 [details] Another build fix Attachment 133601 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12122912
Comment on attachment 133601 [details] Another build fix View in context: https://bugs.webkit.org/attachment.cgi?id=133601&action=review Do we know what the perf effect of this change is? > Source/WebCore/css/StylePropertySet.cpp:730 > + // Don't use CSSPropertyFont because old UAs can't recognize them but are important for editing. Is this really still the case? :)
(In reply to comment #22) > (From update of attachment 133601 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133601&action=review > > Do we know what the perf effect of this change is? I don't think there's any perf. implications because we didn't even use StringBuilder before my patch for https://bugs.webkit.org/show_bug.cgi?id=82028 was landed. > > Source/WebCore/css/StylePropertySet.cpp:730 > > + // Don't use CSSPropertyFont because old UAs can't recognize them but are important for editing. > > Is this really still the case? :) Yes. Unfortunately, some old versions of mail clients support as little CSS as MSIE4 did :( We even used to generate font elements for a very old mail client that didn't support CSS at all.
Comment on attachment 133601 [details] Another build fix View in context: https://bugs.webkit.org/attachment.cgi?id=133601&action=review The patch looks good to me. Thanks for doing this :-) > Source/WTF/wtf/BitVector.h:170 > + Unnecessary empty line.
Comment on attachment 133601 [details] Another build fix Please fix the build breaks. The rest of the patch looks good.
Created attachment 133909 [details] Fixed win build
Comment on attachment 133909 [details] Fixed win build View in context: https://bugs.webkit.org/attachment.cgi?id=133909&action=review r+ with the Linux build fix and the ChangeLog entry. > Source/JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.def:302 > ?retrieveCallerFromVMCode@Interpreter@JSC@@QBE?AVJSValue@2@PAVExecState@2@PAVJSFunction@2@@Z You need a ChangeLog entry for this change.
Comment on attachment 133909 [details] Fixed win build Attachment 133909 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12145156
Comment on attachment 133909 [details] Fixed win build View in context: https://bugs.webkit.org/attachment.cgi?id=133909&action=review > Source/WebCore/css/StylePropertySet.cpp:777 > + case CSSPropertyWebkitMaskPositionX: > + case CSSPropertyWebkitMaskPositionY: > + shorthandPropertyID = CSSPropertyWebkitMaskPosition; > + break; > + case CSSPropertyWebkitMaskRepeatX: > + case CSSPropertyWebkitMaskRepeatY: > + shorthandPropertyID = CSSPropertyWebkitMaskRepeat; Huh, these properties need to be merged with the following properties for CSSPropertyWebkitMask. It's causing LayoutTests/fast/css/remove-shorthand.html to fail now. It seems like something has changed since last week :\
Committed r112177: <http://trac.webkit.org/changeset/112177>
(In reply to comment #30) > Committed r112177: <http://trac.webkit.org/changeset/112177> Is this what caused fast/dom/css-dom-read-2.html to fail?
(In reply to comment #31) > (In reply to comment #30) > > Committed r112177: <http://trac.webkit.org/changeset/112177> > > Is this what caused fast/dom/css-dom-read-2.html to fail? So it seems. Will rebaseline. It's odd that I didn't see this failure locally last week :\
Chromium build fix landed in http://trac.webkit.org/changeset/112185, and rebaselined the test in http://trac.webkit.org/changeset/112188.