The default bug view has changed. See this FAQ.

execCommand("backcolor") should work the same as execCommand("hilitecolor")

RESOLVED FIXED in mozilla14

Status

()

Core
Editor
RESOLVED FIXED
14 years ago
5 years ago

People

(Reporter: Stefan Murariu, Assigned: ayg)

Tracking

(Depends on: 1 bug)

Trunk
mozilla14
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: midas)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.4b) Gecko/20030507

The backcolor command will produce under I.E. the following code <FONT
style="BACKGROUND-COLOR:argument_color">Selected Text</FONT>. Mozilla
just change the background-color property of body for entire page, and cannot be
used getting the innerHTML of this. I see the best option to implement this just
as forecolor i.e. <span style="background-color:argument_color"> which is
working well.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
(Reporter)

Comment 1

14 years ago
Created attachment 123122 [details]
the main editor file

the example of midas use founded on devedge.netscape.com
(Reporter)

Comment 2

14 years ago
Created attachment 123123 [details]
the file to be edited

Initialy the file contain the "How must look" source

Comment 3

14 years ago
-->brade
Assignee: general → brade
Component: Browser-General → Editor: Core
OS: Windows 98 → All
Hardware: PC → All

Comment 4

14 years ago
This is invalid.  use "hilitecolor" in midas to set the hilitecolor (text
background color).  Background color in midas is specific to the background of
the page or a table/table cell.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → INVALID
Whiteboard: midas
Gecko's current behavior seems to be to set bgcolor on the nearest td/body ancestor in non-CSS mode, and background-color on the nearest block ancestor in CSS mode.  This is undesirable for a whole bunch of reasons:

* It doesn't match any other browser.  IE and WebKit both treat backColor basically the same as Gecko's hiliteColor.  (WebKit supports both commands identically; IE doesn't support hiliteColor.)
* The dual behavior of setting bgcolor on the body or td in non-CSS mode is very confusing.
* Setting bgcolor on the body doesn't work at all in contenteditable, only designMode.  Gecko seems to set bgcolor on the body anyway, even if it's not editable, which is downright alarming.
* The behavior in CSS mode doesn't make sense from the user's perspective.  For instance, if each line is wrapped in a div, the background of the selected line will change; but if lines are separated by br, the whole editable area's background will change.  Worse, the background will be put on the contenteditable element instead of its contents, so if the HTML is submitted using innerHTML, the background won't be.  Even if the line's background is highlighted, it won't be the "whole" line if the element has a margin.  Etc.

I think the sanest path here is to follow WebKit, and make backColor an alias for hiliteColor.  This is what the editing spec currently requires:

http://dvcs.w3.org/hg/editing/raw-file/tip/editing.html#the-backcolor-command

I'll submit a patch shortly.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: INVALID → ---
Summary: Midas is changing the background color of entire page whit execCommand backcolor → execCommand("backcolor") should work the same as execCommand("hilitecolor")
Created attachment 615119 [details] [diff] [review]
Patch v1

New richtext2 passes:

1003 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [A, Proposed, BC:blue_TEXT-1_SI, dM] used to fail, but it just started passing - 1 should equal 1
1006 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [A, Proposed, BC:blue_TEXT-1_SI, body] used to fail, but it just started passing - 1 should equal 1
1009 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [A, Proposed, BC:blue_TEXT-1_SI, div] used to fail, but it just started passing - 1 should equal 1
1138 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [AC, Proposed, BC:blue_TEXT-1_SI, dM] used to fail, but it just started passing - 1 should equal 1
1141 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [AC, Proposed, BC:blue_TEXT-1_SI, body] used to fail, but it just started passing - 1 should equal 1
1144 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [AC, Proposed, BC:blue_TEXT-1_SI, div] used to fail, but it just started passing - 1 should equal 1
1243 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:842_FONTs:bc:fca-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1245 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:842_FONTs:bc:fca-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1247 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:842_FONTs:bc:fca-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1249 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:842_FONTs:bc:fca-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1253 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:00f_SPANs:bc:f00-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1255 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:00f_SPANs:bc:f00-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1257 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:00f_SPANs:bc:f00-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1259 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:00f_SPANs:bc:f00-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1263 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:ace_FONT.ass.s:bc:rgb-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1265 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:ace_FONT.ass.s:bc:rgb-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1267 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:ace_FONT.ass.s:bc:rgb-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1269 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [C, Proposed, BC:ace_FONT.ass.s:bc:rgb-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1435 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1437 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1439 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1441 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1445 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SO, dM] used to fail, but it just started passing - 1 should equal 1
1447 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SO, dM] used to fail, but it just started passing - 1 should equal 1
1449 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SO, body] used to fail, but it just started passing - 1 should equal 1
1451 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_SPANs:bc:b-1_SO, body] used to fail, but it just started passing - 1 should equal 1
1461 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1463 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-1_SW, dM] used to fail, but it just started passing - 1 should equal 1
1465 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1467 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-1_SW, body] used to fail, but it just started passing - 1 should equal 1
1471 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-2_SW, dM] used to fail, but it just started passing - 1 should equal 1
1473 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-2_SW, dM] used to fail, but it just started passing - 1 should equal 1
1475 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-2_SW, body] used to fail, but it just started passing - 1 should equal 1
1477 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-2_SW, body] used to fail, but it just started passing - 1 should equal 1
1479 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-2_SW, div] used to fail, but it just started passing - 1 should equal 1
1481 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-2_SW, div] used to fail, but it just started passing - 1 should equal 1
1483 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-3_SO, dM] used to fail, but it just started passing - 1 should equal 1
1485 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-3_SO, dM] used to fail, but it just started passing - 1 should equal 1
1487 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-3_SO, body] used to fail, but it just started passing - 1 should equal 1
1489 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-3_SO, body] used to fail, but it just started passing - 1 should equal 1
1491 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-3_SO, div] used to fail, but it just started passing - 1 should equal 1
1493 ERROR TEST-UNEXPECTED-PASS | /tests/editor/libeditor/html/tests/browserscope/test_richtext2.html | [CC, Proposed, BC:gray_P-SPANs:bc:b-3_SO, div] used to fail, but it just started passing - 1 should equal 1

Plus the new richtext passes.  I checked comm-central for other users of this command and found none.

Try run: https://tbpl.mozilla.org/?tree=Try&rev=07bd16a5ec70

The patch applies on top of bug 279330 and bug 738385 to avoid conflicts in richtext2 updates.
Assignee: brade → ayg
Status: REOPENED → ASSIGNED
Attachment #615119 - Flags: review?(ehsan)
(Try run passed; failures are due to bug 279330.  See also <https://tbpl.mozilla.org/?tree=Try&rev=23b3a61d4ca3>, <https://tbpl.mozilla.org/?tree=Try&rev=0a582f0d148c>.)

Comment 8

5 years ago
Try run for 07bd16a5ec70 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=07bd16a5ec70
Results (out of 220 total builds):
    exception: 2
    success: 163
    warnings: 55
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-07bd16a5ec70
Comment on attachment 615119 [details] [diff] [review]
Patch v1

Review of attachment 615119 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, assuming that you have looked to make sure comm-central is not using backcolor.
Attachment #615119 - Flags: review?(ehsan) → review+
Created attachment 615638 [details] [diff] [review]
Patch v2, leaves cmd_backgroundColor untouched

Oops -- I found an apparent user:

http://mxr.mozilla.org/comm-central/source/editor/ui/composer/content/editor.js#1156

It uses cmd_backgroundColor directly, AFAICT, not execCommand("backcolor").  I found no uses of the latter.  So this patch just changes execCommand("backcolor") to use cmd_highlight and leaves all the cmd_backgroundColor use in place.  This is a significant change from the last patch, so I'll re-request review in case you want me to do something else (like still remove the code and get the comm-central people to remove the callers).
Attachment #615119 - Attachment is obsolete: true
Attachment #615638 - Flags: review?(ehsan)

Comment 11

5 years ago
Try run for 4c4306ac51c6 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4c4306ac51c6
Results (out of 217 total builds):
    exception: 1
    success: 183
    warnings: 33
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ayg@aryeh.name-4c4306ac51c6
Comment on attachment 615638 [details] [diff] [review]
Patch v2, leaves cmd_backgroundColor untouched

Review of attachment 615638 [details] [diff] [review]:
-----------------------------------------------------------------

Please file a follow-up to fix the comm-central caller, and another one to do what your original patch did.  Thanks!
Attachment #615638 - Flags: review?(ehsan) → review+
Depends on: 746165
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ddeb95d7650

Follow-ups: bug 746165, bug 746166.
Flags: in-testsuite+
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/8ddeb95d7650
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago5 years ago
Resolution: --- → FIXED

Updated

5 years ago
No longer depends on: 746165

Updated

5 years ago
Depends on: 746165
You need to log in before you can comment on or make changes to this bug.