Closed Bug 968637 Opened 10 years ago Closed 9 years ago

add the DOMTokenList relList to HTMLLinkElement, HTMLAreaElement and HTMLAnchorElement

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla30

People

(Reporter: karlcow, Assigned: agi)

Details

(Keywords: dev-doc-complete, Whiteboard: [mentor=bz][lang=c++])

Attachments

(1 file, 2 obsolete files)

Added "rel" and "relList" to HTMLAreaElement and "relList" to HTMLLinkElement, HTMLAnchorElement; r=bz
14.04 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review 
A "rel" attribute in a "link", "area" or a "a" element may contain a list of space separated values. Example:

<link rel="shortcut icon" …>
or 
<a href="…" rel="archive month">…</a>

The interface HTMLLinkElement : HTMLElement {…} contains

     readonly attribute DOMTokenList relList;

See http://www.w3.org/html/wg/drafts/html/master/document-metadata.html#dom-link-rellist

and interface HTMLAnchorElement : HTMLElement {…}

contains

  readonly attribute DOMTokenList relList;

};

See http://www.w3.org/html/wg/drafts/html/master/text-level-semantics.html#htmlanchorelement

and interface HTMLAreaElement : HTMLElement {…}

contains

  readonly attribute DOMTokenList relList;

};

See http://www.w3.org/html/wg/drafts/html/master/embedded-content-0.html#htmlareaelement


As of Firefox 27, the method is currently not implemented both for "link", "area" and "a".
Maybe a test.

<!doctype html>
<html>
<title>test link</title>
<link rel="shortcut icon" href="/favicon.ico">

<script>
var linkrellist = document.querySelectorAll('link[rel]');
var link = linkrellist[0];
if (link[0].relList.indexOf('shortcut') && link[0].relList.indexOf('icon')) {
   console.log('pass')
   } else {
   console.log('fail')}
</script>
</html>
s/link[0]/link/
Summary: add the DOMTokenList relList to HTMLLinkElement → add the DOMTokenList relList to HTMLLinkElement, HTMLAreaElement and HTMLAnchorElement
Whiteboard: [mentor=bz][lang=c++]
I would like to work on this. Could you please give me a little guide about this bug?
The simplest thing to do is probably to just add a new member to the relevant classes to store the list, uncomment the relevant lines in the WebIDL, and implement the getter.  The getter will end up doing what Element::GetClassList does, except with a different attr name and storing the value in the element directly, not in the slots.
Could you please be a little more specific? Which part of code should I read? Thank you.
Element::GetClassList can be found via http://dxr.mozilla.org/mozilla-central/search?q=Element%3A%3AGetClassList&redirect=true and is at http://dxr.mozilla.org/mozilla-central/source/content/base/src/Element.cpp#435

The other relevant files are dom/webidl/HTMLLinkElement.webidl, dom/webidl/HTMLAreaElement.webidl, dom/webidl/HTMLAnchorElement.webidl, and content/html/content/src/HTMLLinkElement.{h,cpp} and so forth.
Attached patch relList.patch (obsolete) — — Splinter Review 
I cooked up a patch for HTMLLinkElement and HTMLAnchorElement, for HTMLAreaElement it seems that even "rel" is not implemented, so I figure we should do that first.

About the test, I think it should be 

    if (link.relList.contains('shortcut') && link.relList.contains('icon')) {

instead of

    if (link[0].relList.indexOf('shortcut') && link[0].relList.indexOf('icon')) {

Can I have this bug assigned so I can finish it? Thank you!
Attachment #8384292 - Flags: review?(bzbarsky)
Attachment #8384292 - Attachment is obsolete: true
Attachment #8384292 - Flags: review?(bzbarsky)
I'm sorry I'm still trying to figure out how mercurial works. This new version of the patch has a description and adds rel and relList to HTMLAreaElement as well. 

Please let me know if I have to modify something else, I tried searching through the code but I'm very new here. Thanks.
Attachment #8384392 - Flags: review?(bzbarsky)
Assignee: nobody → agi.novanta
Comment on attachment 8384392 [details] [diff] [review]
Added "rel" to HTMLAreaElement and "relList" to HTMLAreaElement, HTMLLinkElement and HTMLAnchorElement

>+nsRefPtr<nsDOMTokenList >

This return type (unlike the type of the member) should just be nsDOMTokenList*.

>+HTMLAnchorElement::RelList() {

Linebreak before the '{', please.

>+++ b/content/html/content/src/HTMLAnchorElement.h

Please don't make stray whitespace changes (e.g. removing blank lines) here.

You need to add this new member to cycle collection.  Just NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mRelList) in the NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED bit and NS_IMPL_CYCLE_COLLECTION_UNLINK(mRelList) in the NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED bit should do the trick.

Similar issues for the other files.

r=me with those nits fixed, and thank you!
Attachment #8384392 - Flags: review?(bzbarsky) → review+
Giovanni, you plan to update the patch, right?
Flags: needinfo?(agi.novanta)
Yes. I just have to review everything. I'll submit the patch Friday or Saturday. Thank you.
Flags: needinfo?(agi.novanta)
Ok, I think I fixed everything. Please let me know if there's something wrong. Thank you!
Attachment #8384392 - Attachment is obsolete: true
Attachment #8388085 - Flags: review?(bzbarsky)
Comment on attachment 8388085 [details] [diff] [review]
Added "rel" and "relList" to HTMLAreaElement and "relList" to HTMLLinkElement, HTMLAnchorElement; r=bz

r=me
Attachment #8388085 - Flags: review?(bzbarsky) → review+
https://hg.mozilla.org/mozilla-central/rev/64d45543afe4
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.