Add 'sizes' getter/setter to link elements

ASSIGNED
Assigned to

Status

()

Core
DOM
ASSIGNED
9 years ago
6 years ago

People

(Reporter: rflint, Assigned: fabrice)

Tracking

Trunk
mozilla1.9.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 4 obsolete attachments)

 
Created attachment 326656 [details] [diff] [review]
Patch

Noticed this was a frozen interface only after writing the patch. Guess this is a Moz2 kind of thing then?
Attachment #326656 - Flags: superreview?(jst)
Attachment #326656 - Flags: review?(jst)

Comment 2

9 years ago
New things should go to nsIDOMNSHTMLLinkElement interface.
Comment on attachment 326656 [details] [diff] [review]
Patch

Yeah, either wait for Moz2 (and re-request reviews then), or create a new nsIDOMNSHTMLLinkElement interface that extens nsIDOMHTMLLinkElement and adds this property.
Attachment #326656 - Flags: superreview?(jst)
Attachment #326656 - Flags: superreview-
Attachment #326656 - Flags: review?(jst)
Attachment #326656 - Flags: review-
Created attachment 343495 [details] [diff] [review]
Patch v2
Attachment #326656 - Attachment is obsolete: true
Attachment #343495 - Flags: review?(Olli.Pettay)
Created attachment 343496 [details] [diff] [review]
Patch v2.1

Let's try that again with the new interface actually included in the diff this time...
Attachment #343495 - Attachment is obsolete: true
Attachment #343496 - Flags: review?(Olli.Pettay)
Attachment #343495 - Flags: review?(Olli.Pettay)

Comment 6

9 years ago
Comment on attachment 343496 [details] [diff] [review]
Patch v2.1


>+[scriptable, uuid(ea4e4325-9cc4-42ac-8126-0a8c035c165a)]
>+interface nsIDOMNSHTMLLinkElement : nsISupports
>+{
>+  attribute DOMString    sizes;
>+};
Make nsIDOMNSHTMLLinkElement inherit nsIDOMHTMLLinkElement and then
nsHTMLLinkElement should inherit only nsIDOMNSHTMLLinkElement.
That way you don't increase the size of the nsHTMLLinkElement
(doesn't increase vtable).

Comment 7

8 years ago
Comment on attachment 343496 [details] [diff] [review]
Patch v2.1

New patch coming?
Attachment #343496 - Flags: review?(Olli.Pettay) → review-
Target Milestone: mozilla1.9.1 → ---
(Assignee)

Updated

7 years ago
Blocks: 596650
(Assignee)

Updated

7 years ago
Target Milestone: --- → mozilla1.9.1
(Assignee)

Comment 8

7 years ago
Created attachment 480372 [details] [diff] [review]
new patch

New patch, taking advantage of the now non-frozen interfaces to simply add the sizes attribute to nsIDOMHTMLLinkElement
Attachment #480372 - Flags: review?
(Assignee)

Updated

7 years ago
Attachment #480372 - Flags: review? → review?(Olli.Pettay)

Comment 9

7 years ago
Comment on attachment 480372 [details] [diff] [review]
new patch

You should update the iid of the interface.
And where is the test?
Attachment #480372 - Flags: review?(Olli.Pettay) → review-
(Assignee)

Comment 10

7 years ago
Created attachment 480602 [details] [diff] [review]
addression comments

Addressing issues from comment 9
Assignee: rflint → fabrice
Attachment #480372 - Attachment is obsolete: true
Attachment #480602 - Flags: review?(Olli.Pettay)
(Assignee)

Comment 11

7 years ago
Created attachment 480604 [details] [diff] [review]
corrected patch

Sorry, I had not hg added the test case.
Attachment #480602 - Attachment is obsolete: true
Attachment #480604 - Flags: review?(Olli.Pettay)
Attachment #480602 - Flags: review?(Olli.Pettay)
The IDL attribute shouldn't be a DOMString but a DOMSettableTokenList.

By the way, if you just need .sizes done this way (a simple reflection of the content attribute), you can use .getAttribute('sizes') for the moment and change that when .sizes will be implemented.
Comment on attachment 480604 [details] [diff] [review]
corrected patch

Mounir is very true. 
.sizes isn't a DOMString, but DOMSettableTokenList.
Unfortunately the current HTML5 draft has  [PutForwards=value] - 
I think we should get rid of that.
Want to sent question to whatwg why Hixie has used
PutForwards (which is generally an indicator of an ugly API, IMHO)?
Attachment #480604 - Flags: review?(Olli.Pettay) → review-
You need to log in before you can comment on or make changes to this bug.