Closed
Bug 828931
Opened 12 years ago
Closed 2 years ago
HTML+CSS editing rules for inline styles
Categories
(Core :: DOM: Editor, defect, P5)
Core
DOM: Editor
Tracking
()
RESOLVED
DUPLICATE
of bug 1803044
People
(Reporter: glazou, Unassigned)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
15.97 KB,
patch
|
Details | Diff | Splinter Review |
The HTML+CSS editing rules for inline styles in the editor are horked.
Steps to reproduce the bug:
1. open the Midas Demo at http://www-archive.mozilla.org/editor/midasdemo/
2. enter some text in the editable area
3. select some of the text you just entered
4. click on the B button and then on the I button
Expected result, verified in FF12: one span only with two inline styles
Actual result in Nightly 21.0a1 2013-01-09: two nested spans with one inline style each..
This could be a rather old regression.
Impacts Thunderbird for rich-mail composition, BlueGriffon, BlueGriffon EPUB, SeaMonkey.
Updated•12 years ago
|
Keywords: regressionwindow-wanted
Reporter | ||
Comment 1•12 years ago
|
||
Regression from bug 746515, apparently
Reporter | ||
Comment 2•12 years ago
|
||
For the following test:
data:text/html,<!doctype html><div contenteditable><span style="font-style:italic">bar</span></div><script>getSelection().selectAllChildren(document.body.firstChild);document.execCommand("stylewithcss", false, true);document.execCommand("bold");document.body.textContent = document.body.firstChild.innerHTML;</script>
WebKit replies:
<span style="font-style:italic"><span style="font-weight: bold;">bar</span></span>
This seems suboptimal to me. Before the fix for bug 746515, Gecko used to
reply:
<span style="font-style:italic; font-weight: bold;">bar</span>
We now reply:
<span style="font-weight: bold;"><span style="font-style:italic">bar</span></span>
which is as ugly as WebKit's answer but different :-)
What we did in the past was cleaner, simpler and more in line with all the
magic in nsHTMLEditorStyle.cpp.
Reporter | ||
Comment 3•12 years ago
|
||
Found one of the reasons why Aryeh's code is not working, but it's not the only one:
in http://mxr.mozilla.org/mozilla-central/source/editor/libeditor/html/nsHTMLEditorStyle.cpp#317 , the code |element->GetAttrCount() != 1| is
wrong and makes the editor behave badly... Aryeh forgot the mozdirty attribute
so this should be checking
attrCount != 1 and (attrCount !=2 || (no_mozdirty_attribute_on_element))
Reporter | ||
Comment 4•12 years ago
|
||
Reporter | ||
Comment 5•12 years ago
|
||
Better fix. Does exactly what is described above.
Attachment #700430 -
Attachment is obsolete: true
Comment 6•12 years ago
|
||
Can you please generate a patch with 8 lines of context and send this off to try? (We also need a testcase for this patch, of course.)
Reporter | ||
Comment 7•12 years ago
|
||
Ehsan, I have no hg account. Two options here: you vouch for me and get me
one so I can send to try server and ultimately check that in; you do it
yourself ;-)
I think it's highly time for me to regain write access in Gecko.
Attachment #700493 -
Attachment is obsolete: true
Comment 8•12 years ago
|
||
(In reply to comment #7)
> Created attachment 700943 [details] [diff] [review]
> --> https://bugzilla.mozilla.org/attachment.cgi?id=700943&action=edit
> new |-U 8| patch and test
>
> Ehsan, I have no hg account. Two options here: you vouch for me and get me
> one so I can send to try server and ultimately check that in; you do it
> yourself ;-)
> I think it's highly time for me to regain write access in Gecko.
I'll definitely vouch for you! Just file a bug and CC me on it, please.
Comment 9•12 years ago
|
||
(In reply to Daniel Glazman (:glazou) from comment #0)
> The HTML+CSS editing rules for inline styles in the editor are horked.
> Steps to reproduce the bug:
>
> 1. open the Midas Demo at http://www-archive.mozilla.org/editor/midasdemo/
> 2. enter some text in the editable area
> 3. select some of the text you just entered
> 4. click on the B button and then on the I button
>
> Expected result, verified in FF12: one span only with two inline styles
>
> Actual result in Nightly 21.0a1 2013-01-09: two nested spans with one inline
> style each..
This is by design, for consistency between HTML and CSS modes. It's bug 746515 patch part 5, "Only add style="" to empty <span>s". Once the bold adds a span, it's not empty, so italics will add a new span instead of modifying the existing one. See bug 746515 comment 1 for rationale:
> The spec says to only ever add style="" to empty spans. This also makes CSS
> and non-CSS mode more closely analogous -- every time non-CSS mode would add
> an element, so does CSS mode. This is more likely to prevent discrepancies.
> E.g., consider
>
> data:text/html,<!doctype html>
> <div contenteditable><font color=red>foo</font><br><font
> color=red>bar</font></div>
> <script>
> getSelection().selectAllChildren(document.body.firstChild.firstChild);
> document.execCommand("stylewithcss", false, true);
> document.execCommand("underline");
> getSelection().selectAllChildren(document.body.firstChild.childNodes[2]);
> document.execCommand("stylewithcss", false, false);
> document.execCommand("underline");
> getSelection().removeAllRanges();
> </script>
>
> In Gecko, "foo"'s underline is red, while "bar"'s is black. This is because
> CSS mode adds text-decoration: underline to the <font>, while non-CSS mode
> nests it in a <u>, and the color of the underline is the color of the
> element that generates it.
>
> So my patch here is going to make Gecko only ever add style="" to an empty
> span. If the current node isn't an empty span (after removing styles), it
> will get wrapped.
Assuming we do want to keep the spec's behavior, it might be worthwhile to add a test for this case.
Updated•11 years ago
|
Keywords: regressionwindow-wanted
Comment 10•4 years ago
|
||
Bulk-downgrade of unassigned, >=3 years untouched DOM/Storage bug's priority.
If you have reason to believe this is wrong, please write a comment and ni :jstutte.
Severity: major → S4
Priority: -- → P5
Comment 11•2 years ago
|
||
Fixed in bug 1803044.
You need to log in
before you can comment on or make changes to this bug.
Description
•