Closed Bug 661327 Opened 13 years ago Closed 13 years ago

Warn in of to-be-removed Attr functions

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: Ms2ger, Assigned: Ms2ger)

References

(Depends on 2 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 1 obsolete file)

      No description provided.
Flags: in-testsuite-
Attached patch Patch v1Splinter Review
Attachment #539172 - Flags: review?(jonas)
Comment on attachment 539172 [details] [diff] [review]
Patch v1

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

You also need to warn in G/SetAttributeNodeNS

r=me with that fixed.
Attachment #539172 - Flags: review?(jonas) → review+
In dom.properties, the string:
NodeTypeWarning= misses a word at the end of the sentence. We don't know what this always returns.
Can someone be so kind to explain in plain English the meaning of all those warnings starting with "Use of attributes'"? I'm honestly unable to understand them in order to provide a proper translation.
http://hg.mozilla.org/mozilla-central/rev/5028de4c95e9 apparently landed.

+GetAttributeNodeWarning=Use of getAttributeNode() is deprecated. Use getAttribute() instead.
+SetAttributeNodeWarning=Use of setAttributeNode() is deprecated. Use setAttribute() instead.
+GetAttributeNodeWarning=Use of getAttributeNodeNS() is deprecated. Use getAttributeNS() instead.
+SetAttributeNodeWarning=Use of setAttributeNodeNS() is deprecated. Use setAttributeNS() instead.

has ambiguous strings. And yeah, I don't really understand these strings, either.
IOW, those methods [1] are deprecated, that means they shouldn't be used anymore. These warnings are going to be shown (in the error console I believe) when a script is trying to use them. Those methods have replacement (basically, the same name without 'Node') and they should be used instead.

[1] getAttributeNode(), setAttributeNode(), getAttributeNodeNS() and setAttributeNodeNS()

And marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
reopening. The keys for 

GetAttributeNodeWarning=Use of getAttributeNode() is deprecated. Use getAttribute() instead.
GetAttributeNodeWarning=Use of getAttributeNodeNS() is deprecated. Use getAttributeNS() instead.

are the same. This is not gonna work.

Also, the question about non-grokkable messages is really about 

Use of attributes' attributes attribute is deprecated. It always returns null.

and friends.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
http://hg.mozilla.org/mozilla-central/rev/5028de4c95e9

(In reply to comment #3)
> In dom.properties, the string:
> NodeTypeWarning= misses a word at the end of the sentence. We don't know
> what this always returns.

It returns 2. I'll push a followup.

(In reply to comment #4)
> Can someone be so kind to explain in plain English the meaning of all those
> warnings starting with "Use of attributes'"? I'm honestly unable to
> understand them in order to provide a proper translation.

Attributes are objects, and using their "specified", "nodeName", "nodeValue", "attributes", etc. properties (which are also called attributes in this context) is deprecated.

(In reply to comment #7)
> reopening. The keys for 
> 
> GetAttributeNodeWarning=Use of getAttributeNode() is deprecated. Use
> getAttribute() instead.
> GetAttributeNodeWarning=Use of getAttributeNodeNS() is deprecated. Use
> getAttributeNS() instead.
> 
> are the same. This is not gonna work.

Will fix that too.
(In reply to comment #8)
> Attributes are objects, and using their "specified", "nodeName",
> "nodeValue", "attributes", etc. properties (which are also called attributes
> in this context) is deprecated.

So, are these equivalent? 

"Use of attributes' lastChild attribute is deprecated." -> "Use of lastChild on attributes is deprecated." 

"Use of attributes' attributes attribute is deprecated" -> "Use of attributes on attributes is deprecated."
Yes, that sounds good too.
InsertBeforeWarning=Use of attributes' insertBefore() is deprecated. Use value instead.
ReplaceChildWarning=Use of attributes' replaceChild() is deprecated. Use value instead.
RemoveChildWarning=Use of attributes' removeChild() is deprecated. Use value instead.
AppendChildWarning=Use of attributes' appendChild() is deprecated. Use value instead.

These don't look right to me: shouldn't they end with "Use $other_function_name instead"?
Pushed

http://hg.mozilla.org/mozilla-central/rev/42583f997120

for the followups, with r=sicking over IRC.

(In reply to comment #11)
> InsertBeforeWarning=Use of attributes' insertBefore() is deprecated. Use
> value instead.
> ReplaceChildWarning=Use of attributes' replaceChild() is deprecated. Use
> value instead.
> RemoveChildWarning=Use of attributes' removeChild() is deprecated. Use value
> instead.
> AppendChildWarning=Use of attributes' appendChild() is deprecated. Use value
> instead.
> 
> These don't look right to me: shouldn't they end with "Use
> $other_function_name instead"?

They are correct; "value" returns the contents of the text node which is a child node of the attribute, so changing an attribute's "value" and changing its child node are equivalent. (This complexity is what we want to remove, and the reason for this bug.)
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Is it at all possible to provide the actual document and line number where the deprecated function was called? If they're to be removed someone will have to weed them out of the code. (I'm not volunteering, BTW!)
No idea
Could you have not at least removed all of the in-tree callers before warning about them to other people?
Depends on: 667618
Why are these methods and properties deprecated? 

Aren't they part of W3C DOM Level 2 and HTML5? HTML5 spec mentions e.g. getAttributeNode here: http://www.w3.org/TR/html5/apis-in-html-documents.html?
Ignore my last comment, I just read bug 611787 and today's DOM Core draft.
Keywords: dev-doc-needed
Is my goal documenting this to flag them all as deprecated, basically?
It might be enough to flag them all as deprecated. I'd also like to see the rationale and the suggested replacements. From reading the draft, it's my understanding that the functions were not being deprecated, but Attr was being changed not to derive from Node, so I'm not entirely sure what's going on here myself.
They are going away entirely, so something stronger than "deprecated" may be appropriate.
When are they going away?

"Deprecated," in my world, means "This isn't gone yet but it's going to be, so stop using it." So that seems like the perfect word for it.
Attached file test case (obsolete) —
Attachment #560140 - Attachment is obsolete: true
Attachment #560141 - Attachment is obsolete: true
Attachment #560140 - Attachment is obsolete: false
Attachment #560141 - Attachment is obsolete: false
Verified on:
Mozilla/5.0 (Windows NT 6.1; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20100101 Firefox/7.0
Mozilla/5.0 (Windows NT 5.1; rv:7.0) Gecko/20100101 Firefox/7.0
+ Aurora (Fx 8.0a2) & Nightly (Fx 9.0a1).

STR:
1. Save the attached test case and aux file in the same location.
2. Load the test case in a Fx tab.
3. Open the Error console.

Expected Results:
The attached test case uses getAttributeNodeNS(), setAttributeNodeNS(), nodeName, nodeValue and nodeType. Warnings about all these being deprecated are displayed in the Error console.

Actual Results:
Only warnings about uses getAttributeNodeNS(), setAttributeNodeNS() and nodeValue being deprecated are displayed in the Error console.

After loading different pages in Fx tabs, I noticed that only warnings for getAttributeNode(), setAttributeNode(), getAttributeNodeNS(), setAttributeNodeNS() and nodeValue are displayed in the console.

Examples of links I used:
http://www.w3schools.com/dom/tryit.asp?filename=try_dom_removecurrent - should display warning about parentNode being deprecated
http://www.w3schools.com/dom/tryit.asp?filename=try_dom_removetextnode - should display warning about childNodes being deprecated
http://www.w3schools.com/dom/tryit.asp?filename=try_dom_replacechild - should display warning about replaceChild() being deprecated
http://www.w3schools.com/dom/tryit.asp?filename=try_dom_removecurrent - should display warning about removeChild() being deprecated
etc
Hm. Does this apply to all cases of, say, Node.getAttributeNode, or are we looking at only when the node is an attribute node that it's deprecated? I started documenting it as all cases, but now I worry that I may be overdoing this. Can someone please clarify?
Attached file test case
Attachment #560140 - Attachment is obsolete: true
Looks like quickstubs are interfering.
Documentation updated:

https://developer.mozilla.org/En/DOM/Attr

Also mentioned on Firefox 7 for developers.
Depends on: 690120
Depends on: 669400
Depends on: 674437
Depends on: 679684
Depends on: 705110
Well, I still get this warning when using jQuery 1.7.1 in Firefox 10.0.2 on Windows 7 SP1 32bits. Just open Firebug and enable it to "show warnings".

Use of getAttributeNodeNS() is deprecated. Use getAttributeNS() instead.

This is a test code.

<?xml version="1.0" encoding="UTF-8"?>

<!DOCTYPE html>
<html xmlns="http://www.w3.org/1999/xhtml">    
    <head>
        <title>Example Application</title>
        <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.7.1/jquery.min.js" type="text/javascript"></script>
        <script type="text/javascript">
            $(function() {
                alert("hello!");
            });
        </script>
    </head>
    <body>
        <p>Example Application</p>
    </body>
</html>
We should file an evangelism bug on jquery.  Ms2ger?

Also means we probably won't be removing these for a while.
Err, the correct name of the setting in Firebug is "Show JavaScript warnings", in the Console tab.

There are some bug tickets, but they say they cannot reproduce them.

http://bugs.jquery.com/ticket/10735
http://bugs.jquery.com/ticket/10532
Can't reproduce either with that snippet; could this be an extension you have installed?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: