Colorpicker button should evaluate 'color' attribute on construction

RESOLVED FIXED

Status

()

Toolkit
XUL Widgets
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Karsten Düsterloh, Assigned: neil@parkwaycc.co.uk)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

12 years ago
When you create a colorpicker button dynamically and set the 'color' attribute, it gets ignored. Only setting the color property after appending the colorpicker button to the document works.

Current behaviour:

Setting attribute before appending: doesn't work
Setting attribute after appending: doesn't work
Setting property before appending: doesn't work
Setting property after appending: works

Expected behaviour:

All four methods should work. (See also attached testcase.)

The same problem exists for XPFE's colorpicker.
(Reporter)

Comment 1

12 years ago
Created attachment 239942 [details]
Testcase
(Assignee)

Comment 2

12 years ago
(In reply to comment #0)
>All four methods should work.
Well, a) should work, b) may work, c) can't work, d) always works
(Assignee)

Comment 3

12 years ago
Created attachment 240378 [details] [diff] [review]
Proposed patch

Much to my surprise, this does fix case c) - any idea why?
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #240378 - Flags: first-review?(enndeakin)

Comment 4

12 years ago
(In reply to comment #3)
> Created an attachment (id=240378) [edit]
> Proposed patch
> 
> Much to my surprise, this does fix case c) - any idea why?
> 

Because the following code adds a 'color' property on the element, although not the one through XBL which hasn't been attached yet. When appending, the XBL constructor is called which reads this property instead.

    if (asAttribute)
      cp.setAttribute('color', color);
    else
      cp.color = color;
  document.getElementById('content').appendChild(cp);

Comment 5

12 years ago
Comment on attachment 240378 [details] [diff] [review]
Proposed patch

But the patch is still good for fixing case A though
Attachment #240378 - Flags: first-review?(enndeakin) → first-review+
(Assignee)

Comment 6

12 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.