Error message not displayed in Color Picker dialog.

VERIFIED FIXED in mozilla0.9.4



18 years ago
17 years ago


(Reporter: bugzilla, Assigned: cmanske)



Firefox Tracking Flags

(Not tracked)



(2 attachments)



18 years ago
Error: editorShell has no properties
Source File: chrome://editor/content/EdDialogCommon.js
Line: 246

to reproduce:
- open prefs
- go to Composer -> New Page Settting
- select Use custom colors:
- click on normal text
- blank the input field and press ok
- press Cancel

build 20010618

Comment 1

18 years ago
Assignee: beppe → cmanske
Priority: -- → P3
Target Milestone: --- → mozilla0.9.3

Comment 2

18 years ago
Looking into it...

Comment 3

18 years ago
one line fix, removal of js error

reviewed and approved
Keywords: nsBranch

Comment 4

18 years ago
removing nsBRanch and pushing out to 1.0
Keywords: nsBranch
Target Milestone: mozilla0.9.3 → mozilla1.0

Comment 5

17 years ago
changing milestone to 0.9.4
Target Milestone: mozilla1.0 → mozilla0.9.4

Comment 6

17 years ago
This is more than just a JS error. The problem is that we don't know about
editorShell when in the pref dialog, so calling "ShowInputError()" method fails
because it tries to use editorShell's messagebox utility. So the real problem
is that we don't show the error message at all.
Changing Summary to reflect the real problem.
Summary: javascript error in EdDialogCommon.js → Error message not displayed in Color Picker dialog.

Comment 7

17 years ago
So once I fixed the basic problem in ShowInputErrorMessage() method, testing
of the Color Picker dialog revealed other problems:
1. The error message would appear twice: once because of call to onOK()
 from "onOKClick()", but then oncommand calling of onOK() would validated and
 show the error again.
2. Default key shifting from the "Last-picked Color" button to OK button wasn't
happening correctly, or when you did use "Enter" key, you didn't get the 
last-picked color when you should, you would get whatever color was under your
mouse cursor.
So the patch also fixes those problems as well.



17 years ago
Keywords: patch, review
Whiteboard: FIX IN HAND need r=, sr=

Comment 8

17 years ago
Since the arrow keys don't update the colorpicker, the new method
SelectColorByKeypress(aEvent) in the patch should only set the color when 
spacebar is used:
function SelectColorByKeypress(aEvent)
  if (aEvent.keyCode == aEvent.DOM_VK_SPACE)

Comment 9

17 years ago
Whiteboard: FIX IN HAND need r=, sr= → FIX IN HAND need sr=

Comment 10

17 years ago
The one thing that bugs me about the 08/13/01 13:00 patch is the gOnOKCalled 
workaround ... are we working around a toolkit bug? Why don't other dialogs 
exhibit this same called twice behavior, or do they?

Comment 11

17 years ago
They don't 'cause no one else is silly enough to put an "onclick" and "oncommand"
handler on the same button, probably!

Comment 12

17 years ago
Created attachment 47386 [details] [diff] [review]
Updated patch to simplify onclick/oncommand handling

Comment 13

17 years ago
Code was simplified:
Realizing that "oncommand" handler for the OK is called after "onclick",
I could simply make "onclick" clear the LastPicked color info by calling
SetDefaultToOk(). This removed need for hacky "gOnOKCalled" and onOkClick() 
Note that the code in EdDialogCommon.js is not longer needed, since it is
superceded by the fix for bug 96649, which fixed the error message reporting.
Whiteboard: FIX IN HAND need sr= → FIX IN HAND need r=, sr=

Comment 14

17 years ago
Remove keycode == 0 stuff; use charcode instead of keycode for space character.  
If the comparison isn't removed, we'll also trigger the code when typing letters, 
numbers, etc.  Remove both comment lines in that function.

r=brade only with those changes

Comment 15

17 years ago
The 08/28/01 15:15 looks much cleaner! Thanks for losing the gOnOKCalled hack. with brade's provisions above.

Comment 16

17 years ago
Everything done as requested, here's the new method:
function SelectColorByKeypress(aEvent)
  if (aEvent.charCode == aEvent.DOM_VK_SPACE)
Whiteboard: FIX IN HAND need r=, sr= → FIX IN HAND reviewed
a=roc+moz on behalf of drivers

Comment 18

17 years ago
checked in
Last Resolved: 17 years ago
Keywords: patch, review
Resolution: --- → FIXED
Whiteboard: FIX IN HAND reviewed

Comment 19

17 years ago
Verified on 8-30 Build
You need to log in before you can comment on or make changes to this bug.