Closed
Bug 289625
Opened 19 years ago
Closed 19 years ago
color pickers in mail compose don't mirror focused color
Categories
(Core :: DOM: Editor, defect)
Core
DOM: Editor
Tracking
()
RESOLVED
FIXED
People
(Reporter: allltaken, Assigned: mkaply)
References
Details
(Keywords: fixed1.8)
Attachments
(1 file)
947 bytes,
patch
|
Brade
:
review+
tor
:
superreview+
asa
:
approval1.8b4+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050407 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b2) Gecko/20050407 The text and background color pickers set the colors ok, but the color picker boxes on the formatting toolbar don't change to reflect the colors that have focus. This bug evidently occurred in the past few days, since the April 2 nightly. Reproducible: Always Steps to Reproduce: 1. Open a mail compose window 2. Set text color 3. Set background color Actual Results: The colors of the picker boxes don't change, not even after moving the cursor to the body area and starting to compose. The text color can be changed, you can type in a new color, move the cursor back to the area of the first color and type in the first color, but the picker box doesn't change. In some circumstances I haven't determined, the text color gets set to the color of the toolbar (grey in this case). Expected Results: The color picker boxes should reflect the colors that have focus.
I should have said, "The text picker box gets set to the toolbar color," not the text color. Sorry.
I found the following by trying a few saved builds. Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.6) Gecko/20050315 has working color picker that matches color settings Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050217 color picker doesn't match color settings. I should have seen this in 1.8x sooner.
Yes, this color picker bug is reproducible for me with Mozilla 1.8b1, but not with Mozilla 1.7.7RC (or Thunderbird 1.0.2), so it seems this is a regression. BTW, I don't think this belongs in the Moz. App. Suite / Composer component, maybe something like Core / MailNews:Composition would be better?
I wasn't sure how to categorize it. The picker failure is in the html composer window (I just checked) as well as the mail/news editor window, so a core editor category sounds good to me. I'm not an expert on component categories.
(In reply to comment #0) > In some circumstances I haven't determined, the text picker box gets set to > the color of the toolbar (grey in this case). When I select text with two different text colors the text picker box switches to transparent (this seems logical) and I get this error in the JS console: Error: Expected color but found 'mixed'. Error in parsing value for property 'background-color'. Declaration dropped. Source File: chrome://editor/content/editor.xul Line: 0 The box doesn't switch back to the right (or wrong) color when I deselect the text. (In reply to comment #4) > The picker failure is in the html composer > window (I just checked) as well as the mail/news editor window You're right, the color picker colors are also not updated in Composer. So maybe Core:Editor (see http://www.mozilla.org/editor/) would be the right place, but I am no expert either...
Assignee: composer → mozeditor
Component: Composer → Editor
Product: Mozilla Application Suite → Core
QA Contact: bugzilla
Version: unspecified → Trunk
Comment 6•19 years ago
|
||
Can you use builds on http://archive.mozilla.org to narrow down when this broke? Note that this isn't really the "right" component for this bug, but we have no "right" one.... :( Requesting blocking-aviary, since presumably tbird has the same issue...
Flags: blocking-aviary1.1?
I'm on dialup so downloading takes time (40 minutes or so). I could do it if nobody else has local files or broadband, but it would take a while.
The picker problem is in Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a4) Gecko/20040927. 1.8a1 should be the next one to try. All the 1.7 versions seem to have good color pickers.
Comment 9•19 years ago
|
||
I have local builds and can try to narrow down, but I'll need detailed instructions on how to reproduce (of the "click here, then click here, then click here" sort) -- comment 0 isn't enough to really sort out what's going on for someone who's never used the HTML mail compose...
Reporter | ||
Comment 10•19 years ago
|
||
My settings: In Mail-News composition settings, set "compose in html". In Preferences, I've got "send in both html and plain text set." The latter shouldn't make any difference as far as composing is concerned. Then open a mail compose window or Composer window. Click on the body area, and then try to set the background and/or text color. You don't need to fill in the address, etc first. Then see whether the background picker changes, and whether the text picker mirrors the color of the text where the cursor is. The same picker behavior seems to be used in both mail-compose and Composer.
Comment 11•19 years ago
|
||
> then try to set the background and/or text color Steps? > and whether the text picker mirrors the color of the text where the cursor is. So I should move the cursor around? Or what?
Reporter | ||
Comment 12•19 years ago
|
||
Click on the body area to make that the focus. Then click on the little white box to bring up a color selection menu for the background. Click on the little black box that's offset from the white one to set the text color from the color selection menu that's evoked as previously. Then type something, note that the color has been selected but is not mirrored in the text-color box (the offset black one), and the background color isn't either. The background picker box remains white---unless you get lucky and find a version that works. :-)
Reporter | ||
Comment 13•19 years ago
|
||
Ok, to be really precise, the color picker boxes are on the Text Formatting toolbar between the font selector and the left A+ symbol that reduces the font size.
Reporter | ||
Comment 14•19 years ago
|
||
In Mozilla 1.8a1 for Win32, the color pickers seemed to work about right. I think they may not have mirrored the active colors until the user started typing, but at least then they did. So it broke sometime between 1.8a1 and 1.8a4.
Comment 15•19 years ago
|
||
John, thank you for the steps! This broke between build 2004-06-14-07 and build 2004-06-15-09. Checkin list for that range plus 2 hours on each side: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-06-14+05%3A00%3A00&maxdate=2004-06-15+11%3A00%3A00&cvsroot=%2Fcvsroot
Comment 16•19 years ago
|
||
This is a regression from bug 206716. See bug 206716 comment 25 for what I suspect the problem is.
Assignee: mozeditor → mozilla
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.8b3?
Comment 17•19 years ago
|
||
[The following comment is also posted in bug 206716 which is currently resolved fixed. I am putting it here because I'm guessing that the fix/patch will be in this bug.] (Disclaimer: I don't have a tree to test this theory right now) The problem with ComposerCommands.js is that goUpdateCommandState() can call pokeMultiStateUI() and cmdParams may have a cstring or a string. doStatefulCommand() also calls pokeMultiStateUI() but cmdParams is always consistent there. One solution would be to make all commands pass the same string type (get rid of all cstrings). Another option is to make pokeMultiStateUI() able to handle either type (instead of calling getStringValue() it could call getValueType() and then call the appropriate string getter method).
OS: Windows 98 → All
Hardware: PC → All
Comment 18•19 years ago
|
||
> One solution would be to make all commands pass the same string type This is probably the better long-term solution, imo. > Another option is to make pokeMultiStateUI() able to handle either type This is probably safer short-term....
Reporter | ||
Comment 19•19 years ago
|
||
Suggestion as to what the resulting behavior should look like: It would look better to the user for color settings including updates of the pickers to go into effect immediately instead of waiting for chars to be typed, as occurred before the breakage.
Assignee | ||
Comment 20•19 years ago
|
||
Minimum patch - use GetValueType to decide what type to get.
Assignee | ||
Updated•19 years ago
|
Attachment #183832 -
Flags: review?(brade)
Comment 21•19 years ago
|
||
Comment on attachment 183832 [details] [diff] [review] Use getvaluetype The patch is fine (too bad the enum types don't better match the api: eStringType for getCStringValue and eWStringType for getStringValue). Please change the curly braces to match the rest of the file: * only use curly braces when more than one line * open curly braces go on their own line Thanks!
Attachment #183832 -
Flags: review?(brade) → review+
Assignee | ||
Comment 22•19 years ago
|
||
Do you think my logic is correct? default to wide string unless it is a CString? Or should I add a wide string check and default to something else?
Comment 23•19 years ago
|
||
I think the logic is fine. I'm not really sure what the appropriate behavior/response should be if a command comes through that doesn't return a string. I believe what happens today with this patch is fine. It wouldn't hurt to do the right thing (do the explicit check for wstring) but I'm not sure it's worth checking for all of the types. Maybe add a comment?
Comment 24•19 years ago
|
||
ping? is this patch ready to land?
Flags: blocking1.8b4?
Flags: blocking1.8b3?
Flags: blocking1.8b3-
Assignee | ||
Updated•19 years ago
|
Attachment #183832 -
Flags: superreview?(tor)
Comment 25•19 years ago
|
||
The patch is waiting for sr...
Updated•19 years ago
|
Flags: blocking-aviary1.1?
Updated•19 years ago
|
Flags: blocking1.8b4? → blocking1.8b4+
Attachment #183832 -
Flags: superreview?(tor) → superreview+
Assignee | ||
Updated•19 years ago
|
Attachment #183832 -
Flags: approval1.8b4?
Updated•19 years ago
|
Attachment #183832 -
Flags: approval1.8b4? → approval1.8b4+
Assignee | ||
Comment 26•19 years ago
|
||
on the branch. waiting for trunk to open.
Assignee | ||
Comment 27•19 years ago
|
||
Fix checked in to trunk.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 28•19 years ago
|
||
In Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050825 SeaMonkey/1.0a, the text picker is updated immediately on selecting a color, but the background picker isn't updated until chars are typed.
Comment 29•19 years ago
|
||
Is that a regression from after this bug was fixed, or has it been a problem all along?
Reporter | ||
Comment 30•19 years ago
|
||
The delayed effectiveness of a color setting was there all along, at least in previous months and perhaps much longer. See my comment #19.
Comment 31•19 years ago
|
||
Probably worth filing a separate bug on...
You need to log in
before you can comment on or make changes to this bug.
Description
•