The default bug view has changed. See this FAQ.

option and optgroup should match enabled/disabled too

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Allan Beaufour, Assigned: Allan Beaufour)

Tracking

({fixed1.8})

Trunk
fixed1.8
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8b5 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

12 years ago
I missed that option and optgroup also have a disabled attribute.
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

12 years ago
Blocks: 306621
Blocks: 84400
No longer depends on: 84400
(Assignee)

Comment 1

12 years ago
Created attachment 194563 [details] [diff] [review]
Patch

Here's a patch.

It basically just adds the AfterSetAttr() functionality to OptGroup and Option.
It's exactly the same code... should I inject a common superclass, or just let
it rest?

We could possibly inject the Before/AfterSetAttr somewhere higher in the object
hierachy, as it is needed in quite a few places now.
Attachment #194563 - Flags: superreview?(bzbarsky)
Attachment #194563 - Flags: review?(bzbarsky)
(Assignee)

Comment 2

12 years ago
Testcases here:
http://beaufour.dk/tmp/
> We could possibly inject the Before/AfterSetAttr somewhere higher in the object
> hierachy

Separate bug on that, please?
Comment on attachment 194563 [details] [diff] [review]
Patch

>Index: content/html/content/src/nsHTMLOptGroupElement.cpp
>+  virtual void AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,

Why virtual?

>Index: content/html/content/src/nsHTMLOptionElement.cpp
>+  virtual void AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,

Again.

Looks great otherwise.
Attachment #194563 - Flags: superreview?(bzbarsky)
Attachment #194563 - Flags: superreview+
Attachment #194563 - Flags: review?(bzbarsky)
Attachment #194563 - Flags: review+
(Assignee)

Updated

12 years ago
Attachment #194563 - Flags: approval1.8b5?

Updated

12 years ago
Attachment #194563 - Flags: approval1.8b5? → approval1.8b5+

Comment 5

12 years ago
please add the fixed1.8 keyword when you land this on the branch
Flags: blocking1.8b5+
(Assignee)

Comment 6

12 years ago
Checked in to 1.8 branch
Keywords: fixed1.8
(Assignee)

Comment 7

12 years ago
Attachment 194563 [details] [diff] does not apply on trunk anymore. Maybe we should wait for bug
308270 before putting this on trunk?
Depends on: 308270
It might be a while before that happens.  Please land this on trunk ASAP so it
can get tested by trunk users too, not just branch users...
(Assignee)

Comment 9

12 years ago
Checked in to trunk
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.