Closed Bug 968637 Opened 12 years ago Closed 12 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: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: