Bug 53465 - Can't remove style when selection starts with a horizontal rule
Summary: Can't remove style when selection starts with a horizontal rule
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-31 17:57 PST by Emerick Rogul
Modified: 2017-07-18 08:30 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Emerick Rogul 2011-01-31 17:57:35 PST
When the current selection includes a horizontal rule and underlined|strike-through text, it's impossible to remove the underlined|strike-through style.  For example, if the selection contains the following:

<hr><u>foo</u>

it's impossible to remove the underline by running execCommand('underline').  In EditorCommand::executeToggleStyleInList(), selectedCSSValue->cssText() returns "none" in this scenario, which causes this method to think it needs to toggle the style on.  It seems that selectedCSSValue->cssText() should be returning "underline" instead.
Comment 1 Emerick Rogul 2011-02-01 10:09:26 PST
Hi Ryosuke,

I've added you to the CC list for this bug, because I noticed that you had last modified EditorCommand::executeToggleStyleInList().  If that's inappropriate, please let me know - I just thought you might have some insight into this problem.
Comment 2 Ryosuke Niwa 2011-03-09 12:23:14 PST
(In reply to comment #0)
> When the current selection includes a horizontal rule and underlined|strike-through text, it's impossible to remove the underlined|strike-through style.  For example, if the selection contains the following:
> 
> <hr><u>foo</u>

More precisely, when the selection starts with hr.  When he appears anywhere else, we can remove / apply text decorations properly.  The problem isn't restricted to text decorations actually. bold, italic, etc... won't work either.

It seems like we're somehow failing to stylize hr.  In the case of br, we can remove the style because we can apply style (e.g. underline results in <u><br>foo</u>).  But then hr isn't a phrasing content so we shouldn't be wrapping it with u, b, and other phrasing elements.  I'll have to look into it.

> it's impossible to remove the underline by running execCommand('underline').  In EditorCommand::executeToggleStyleInList(), selectedCSSValue->cssText() returns "none" in this scenario, which causes this method to think it needs to toggle the style on.  It seems that selectedCSSValue->cssText() should be returning "underline" instead.

I'm not sure if we should be returning underline in this case since the selection starts at hr and hr isn't underlined.
Comment 3 Aryeh Gregor 2011-08-19 14:07:02 PDT
My spec pays attention only to editable text nodes when determining the state/value of inline commands.  E.g.:

"""
If a command has inline command activated values defined, its state is true if either no editable Text node is effectively contained in the active range, and the active range's start node's effective command value is one of the given values; or if there is at least one editable Text node effectively contained in the active range, and all of them have an effective command value equal to one of the given values.
"""
http://aryeh.name/spec/editing/editing.html#inline-command-activated-values

So in the selection {<hr><u>foo</u>}, the only editable text node effectively contained in the selection is "foo", and that's underlined, so queryCommandState("underline") is true.  This works well for inline style commands, since the styles are mostly only relevant to text nodes, and we can only change editable nodes so those are the only ones we should care about.

It might make sense to also care about <br>, but <hr> should almost definitely be ignored here.