Typing style shouldn't inherit style from atomic elements #3675
Conversation
Typing style shouldn't inherit style from atomic elements https://bugs.webkit.org/show_bug.cgi?id=119116 Reviewed by NOBODY (OOPS!). Merge - https://chromium.googlesource.com/chromium/blink/+/27371d2980715475f3443f7025141ebff8537179 This patch changes initialization of typing style not to populate from form control like elements, which can't have range ending point, e.g. img, input, object, textarea, and so on. *Source/WebCore/editing/DeleteSelectionCommand.cpp (DeleteSelectionCommand::initializePositionData()) - Updated to add static variable. (DeleteSectionCommand::saveTypeStyleSheet()) - Added if condition to by using static variable to not inherit style from atomic elements. *LayoutTests/editing/inserting/insert-without-style.html - Testcase added from Blink / Chromium patch. * LayoutTests/editing/inserting/insert-without-style-expected.txt- Added Testcase Expectations.
| // We don't want to inherit style from an element which don't have contents. | ||
| static bool shouldNotInheritStyleFrom(const Node& node) | ||
| { | ||
| return !node.canContainRangeEndPoint(); | ||
| } |
There was a problem hiding this comment.
This looks to be indented wrong. Maybe you have a smart editor that wants to indent everything in the WebCore namespace?
There was a problem hiding this comment.
Yes - I will fix it. Thanks!
| @@ -420,6 +426,9 @@ void DeleteSelectionCommand::saveTypingStyleState() | |||
| return; | |||
| } | |||
|
|
|||
| if (shouldNotInheritStyleFrom(*m_selectionToDelete.start().anchorNode())) | |||
There was a problem hiding this comment.
I understand the Chromium project already made this change, but I think it might not be entirely correct.
Makes sense that we would not want to inherit from, say, an image element but wouldn’t we want to inherit from what’s before/after that element? Like if the text after the image that we were deleting was bold? Test case might be something like either of these:
<b>This is bold <img></b> and this is not
<b>This is bold <img> yes, bold</b> and this is not
Deleting a range starting before the image and ending after the word "and". We’d want the typing style to be bold in both cases, I think. Maybe this isn’t the right test case. I’d like to see a lot more test cases to be sure we got this right.
There was a problem hiding this comment.
Right, this is precisely why I stated Blink fix is wrong in https://bugs.webkit.org/show_bug.cgi?id=119116
webkit-ews-buildbot
added
the
merging-blocked
|
Unfortunately, I am getting weird error on my GitHub fork and I have delete it and add again. I am closing this PR right now and will look into once I fix my Git errors. Thanks! |
954effc