89700 - Setting font color to "" doesn't remove font color attribute

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[scalkins]

Sorry if this is a dupe, but I couldn't find this bug
See this on branch and trunk with todays builds (7/6)
All platforms

Steps to repro:
1)Launch composer
2)Type some text
3)Change the color of subsequent text you type from default to some other color
via selecting it, under the Format-->Text color menu
4)Verify it applies the new color for new text typed
5) Go back to Format-->Text color menu, click the Default button. Then click OK
6)Type some more text

Actual results:The default color does not apply, you still see the new color
when typing text.
 The same goes for changing colors via the text and background icons on the
editor bar. 

This affects IM as well, where it's more visible.

Comment 1

[rubydoo123]

sure enough, the default selection is not working correctly.

Comment 2

[Kathleen :Brade]

In the html source I see that we are adding: <font color="">

We might want to release note that this button doesn't work.  Workaround is to 
use Remove Text Styles (or Discontinue Text Styles) and re-apply any styles that 
are desired.

Comment 3

[Charles Manske]

Something has changed in the rules code such that setting color to "" no longer
removes the color.
But this is easy to fix in JS. Patch comming.

Comment 4

[Charles Manske]

Attachment: Use "RemoveTextAttribute" when clearing text color

Comment 5

[Charles Manske]

I would argue that the suggested JS change is much better than the previous
code that assumed setting font color to empty string should remove the attribute,
i.e., we should be using "RemoveTextAttribute" instead.
This is an important useability issues, so we should make this fix for 6.1.

r=akkana for the fix.

Comment 6

[kinmoz]

sr=[e-mail]

Comment 7

[Charles Manske]

Per discussion with beppe, setting nsBranch.

Comment 8

[Charles Manske]

checked into trunk

Comment 9

[rubydoo123]

adding nsBranch+

Comment 10

[sujay]

this is still not fixed in today's 7/11 trunk build.

Comment 11

[sujay]

I take that back.

THis is fixed on 7/11 trunk. it is indeed working for me on the trunk..

Removing vtrunk keyword.

Lets get this puppy in on the branch.

Comment 12

[Charles Manske]

PDT+ approved by Steve Elmer.

Comment 13

[Charles Manske]

Checked into MOZILLA_0_9_2_BRANCH.

Comment 14

[sujay]

verified in 7/13 branch build on windows.