Closed Bug 969201 Opened 6 years ago Closed 6 years ago

add the DOMSettableTokenList sizes to HTMLLinkElement

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: karlcow, Assigned: baku)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Currently the "sizes" attribute is not exposed in the DOM.

HTML Example:
<link rel="shortcut icon" type="image/x-icon" href="/favicon.ico" sizes="16x16 24x24 32x32 48x48">


IDL interface. See http://www.w3.org/html/wg/drafts/html/master/document-metadata.html#htmllinkelement 

interface HTMLLinkElement : HTMLElement {
  …  
  [PutForwards=value] readonly attribute DOMSettableTokenList sizes;
};


Currently to access the values you need to do:

    var linkrellist = document.querySelectorAll('link[rel*="icon"]');

then loop over linkrellist, for example for a normalized Array of values.

    linkrellist[0].getAttribute('sizes').trim().split(/\s+/);

instead of 

    linkrellist[0].sizes
Attached patch link.patch (obsolete) — Splinter Review
Attachment #8379992 - Flags: review?(Ms2ger)
Comment on attachment 8379992 [details] [diff] [review]
link.patch

Review of attachment 8379992 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/test/test_link_sizes.html
@@ +1,4 @@
> +<!doctype html>
> +<html>
> +<head>
> +<title>Test link.sizes attribute</title>

This is a pretty poor test... Do we have classList tests you could reuse?
Attachment #8379992 - Flags: review?(Ms2ger) → feedback+
Attached patch link.patch (obsolete) — Splinter Review
Ok. Here I tested a bit more how the .sizes attribute works.
Attachment #8379992 - Attachment is obsolete: true
Attachment #8380022 - Flags: review?(Ms2ger)
Actually the spec says that it should be available only if the rel is 'icon'.
Does it mean to return an empty sizes object?
As of today's version, the spec prose is:

> The sizes attribute is used with the icon link type. The attribute must not be specified on link elements that do not have a rel attribute that specifies the icon keyword.

My impression would be that if the icon is not in there, "sizes" would not exist. 

To test for Web compatibility, are there "sizes" attribute out there with "apple-touch-icon" but without "icon" for example.
Your impression is incorrect. See <http://www.whatwg.org/html/#conformance-classes> about the difference between authoring and user agent conformance requirements.
(In reply to :Ms2ger from comment #6)
> Your impression is incorrect. See
> <http://www.whatwg.org/html/#conformance-classes> about the difference
> between authoring and user agent conformance requirements.

hehe. Ms2ger, which parts of the conformance classes of product you wanted the ex-conformance dude to read ;) 

This bit I guess

> User agents that support scripting must also be conforming implementations of the IDL fragments in this specification, as described in the Web IDL specification. [WEBIDL] 

and the following note. So indeed. :) my bad.
Comment on attachment 8380022 [details] [diff] [review]
link.patch

Review of attachment 8380022 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/HTMLLinkElement.cpp
@@ +193,5 @@
> +  if (aNamespaceID == kNameSpaceID_None) {
> +    if (aAttribute == nsGkAtoms::crossorigin) {
> +      ParseCORSValue(aValue, aResult);
> +      return true;
> +    } else if (aAttribute == nsGkAtoms::sizes) {

else if -> if
Attachment #8380022 - Flags: review?(bzbarsky)
Attachment #8380022 - Flags: review?(Ms2ger)
Attachment #8380022 - Flags: feedback+
Comment on attachment 8380022 [details] [diff] [review]
link.patch

In addition to what ms2ger said, you need to add this atom to sPropertiesToTraverseAndUnlink unless you want some fatal asserts when this property is accessed.

Worth adding a test for the putforwards bit.

r=me with that
Attachment #8380022 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/6e5a1ddf0a13
Assignee: nobody → amarchesini
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.