Bug 122874 - During editing, merge inline style with overriding other author styles
Summary: During editing, merge inline style with overriding other author styles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: KyungTae Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-15 18:07 PDT by KyungTae Kim
Modified: 2013-10-16 10:34 PDT (History)
4 users (show)

See Also:


Attachments
testcase (340 bytes, text/html)
2013-10-15 18:07 PDT, KyungTae Kim
no flags Details
Patch (2.29 KB, patch)
2013-10-15 18:16 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (469.26 KB, application/zip)
2013-10-15 19:10 PDT, Build Bot
no flags Details
Patch (4.79 KB, patch)
2013-10-15 22:20 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion (477.61 KB, application/zip)
2013-10-15 23:28 PDT, Build Bot
no flags Details
Patch (6.46 KB, patch)
2013-10-15 23:56 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff
Patch (4.55 KB, patch)
2013-10-16 01:06 PDT, KyungTae Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description KyungTae Kim 2013-10-15 18:07:56 PDT
Created attachment 214326 [details]
testcase

On editable contents, If you backspace before a first character of a line, the line is merged to the upper line with merging parents' style to the inline style.
If there are conflicting styles in parent's styles, not the inline style of the parent but the embedded style of the parent is applied (as the attached test case).
Comment 1 KyungTae Kim 2013-10-15 18:16:49 PDT
Created attachment 214327 [details]
Patch
Comment 2 Build Bot 2013-10-15 19:10:22 PDT
Comment on attachment 214327 [details]
Patch

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

New failing tests:
editing/deleting/merge-paragraph-from-p-with-style.html
editing/deleting/merge-paragraph-with-style-from-rule.html
Comment 3 Build Bot 2013-10-15 19:10:24 PDT
Created attachment 214330 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 4 Ryosuke Niwa 2013-10-15 19:12:58 PDT
Comment on attachment 214327 [details]
Patch

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

This patch needs a test case demonstrating it fixes a bug; at minimum it shouldn't regress tests.

> Source/WebCore/ChangeLog:9
> +        Inline styles need to override other author styles,
> +        so if DoNotOverrideValues mode, inline styles need to be merged before other author styles.

Why?
Comment 5 KyungTae Kim 2013-10-15 22:20:53 PDT
Created attachment 214339 [details]
Patch
Comment 6 KyungTae Kim 2013-10-15 22:26:44 PDT
(In reply to comment #4)
> (From update of attachment 214327 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=214327&action=review
> 
> This patch needs a test case demonstrating it fixes a bug; at minimum it shouldn't regress tests.
> 
> > Source/WebCore/ChangeLog:9
> > +        Inline styles need to override other author styles,
> > +        so if DoNotOverrideValues mode, inline styles need to be merged before other author styles.
> 
> Why?
I added patch with newly added test case.
On the attached test case,
if you backspace on the start of the 2nd line,
the style for class (color:red) is merged,
but the  inline style (color:blue) is not merged because it's DoNotOverrideValues mode.
Then, as a result, only the style that was not applied is merged, and the color is changed!
I think merging inline style first can be a solution for that.
Comment 7 Build Bot 2013-10-15 23:28:21 PDT
Comment on attachment 214339 [details]
Patch

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

New failing tests:
editing/deleting/merge-paragraph-from-p-with-style.html
editing/deleting/merge-paragraph-with-style-from-rule.html
Comment 8 Build Bot 2013-10-15 23:28:22 PDT
Created attachment 214342 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 9 KyungTae Kim 2013-10-15 23:56:52 PDT
Created attachment 214343 [details]
Patch
Comment 10 KyungTae Kim 2013-10-16 01:06:13 PDT
Created attachment 214347 [details]
Patch
Comment 11 Ryosuke Niwa 2013-10-16 10:09:31 PDT
New patch looks much better.
Comment 12 WebKit Commit Bot 2013-10-16 10:34:20 PDT
Comment on attachment 214347 [details]
Patch

Clearing flags on attachment: 214347

Committed r157522: <http://trac.webkit.org/changeset/157522>
Comment 13 WebKit Commit Bot 2013-10-16 10:34:23 PDT
All reviewed patches have been landed.  Closing bug.