746165 - Remove all vestiges of cmd_backgroundColor

Status: NEW

(Core :: DOM: Editor, enhancement, P5)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

We would like to remove cmd_backgroundColor completely, but comm-central is still using it.  See bug 205485 comment 10.  Once that's fixed (followup bug to be filed shortly), my earlier patch from that bug can be applied to remove all the remnants of the command, since it's no longer web-accessible.

Comment 1

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch v1

Obviously, I won't land this till bug 746166 is fixed.  Try run: https://tbpl.mozilla.org/?tree=Try&rev=c752e87c0c79

Comment 2

[(no longer active)]

Comment on attachment 624694 [details] [diff] [review]
Patch v1

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

r=me with the below comment addressed.

::: editor/idl/nsIHTMLEditor.idl
@@ -347,5 @@
> -   * getFontColorState returns what font face is in the selection.
> -   * @param aMixed     True if there is more than one font color
> -   * @return           Color string. "" is returned for none.
> -   */
> -  AString getBackgroundColorState(out boolean aMixed);

Please revise the IID of nsIHTMLEditor.

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Blech, I always forget that!

Comment 4

[[e-mail]]

Comment on attachment 624694 [details] [diff] [review]
Patch v1

>-  NS_REGISTER_ONE_COMMAND(nsBackgroundColorStateCommand, "cmd_backgroundColor");
The patch in bug 746166 removes the use of cmd_backgroundColor...

>-  AString getBackgroundColorState(out boolean aMixed);
... which is the only consumer of getBackgroundColorState...

>-  void setBackgroundColor(in AString aColor);
... however it doesn't remove Composer's use of setBackgroundColor.

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Patch v2, keeps setBackgroundColor

Sad -- only -119 lines now.  Oh well, still an improvement.  Does this work for you?  (Carrying forward Ehsan's review.)

New try run to be on the safe side: https://tbpl.mozilla.org/?tree=Try&rev=f72b65268967

Comment 6

[[e-mail]]

There's a bug in my patch for bug 746166, I'm not sure if it's fixable...

Comment 7

[[e-mail]]

It turns out that I didn't properly replace GetCSSBackgroundColorState...

Comment 8

[Jens Stutte [:jstutte]]

Bulk-downgrade of unassigned, untouched DOM/Storage bug's priority.

If you have reason to believe, this is wrong, please write a comment and ni :jstutte.