Last Comment Bug 661327 - Warn in of to-be-removed Attr functions
: Warn in of to-be-removed Attr functions
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: :Ms2ger
:
Mentors:
Depends on: 674437 679684 667618 669400 690120 705110
Blocks: AttrExodus
  Show dependency treegraph
 
Reported: 2011-06-01 12:16 PDT by :Ms2ger
Modified: 2012-02-23 10:26 PST (History)
17 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (19.85 KB, patch)
2011-06-14 04:34 PDT, :Ms2ger
jonas: review+
Details | Diff | Review
test case (1.24 KB, text/html)
2011-09-14 06:24 PDT, Ioana (away)
no flags Details
test case auxiliary file (509 bytes, text/xml)
2011-09-14 06:25 PDT, Ioana (away)
no flags Details
test case (1.16 KB, text/html)
2011-09-17 05:20 PDT, :Ms2ger
no flags Details

Description :Ms2ger 2011-06-01 12:16:56 PDT

    
Comment 1 :Ms2ger 2011-06-14 04:34:18 PDT
Created attachment 539172 [details] [diff] [review]
Patch v1
Comment 2 Jonas Sicking (:sicking) PTO Until July 5th 2011-06-21 22:16:32 PDT
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.
Comment 3 Cédric Corazza 2011-06-23 11:27:14 PDT
In dom.properties, the string:
NodeTypeWarning= misses a word at the end of the sentence. We don't know what this always returns.
Comment 4 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2011-06-23 12:14:18 PDT
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.
Comment 5 Axel Hecht [:Pike] 2011-06-23 12:51:24 PDT
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.
Comment 6 Mounir Lamouri (:mounir) 2011-06-23 14:56:28 PDT
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.
Comment 7 Axel Hecht [:Pike] 2011-06-23 15:05:24 PDT
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.
Comment 8 :Ms2ger 2011-06-24 01:09:09 PDT
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.
Comment 9 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2011-06-24 01:23:35 PDT
(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."
Comment 10 :Ms2ger 2011-06-24 01:25:40 PDT
Yes, that sounds good too.
Comment 11 Francesco Lodolo [:flod] - OFFLINE Jun 26-Jul 3 2011-06-24 02:12:13 PDT
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"?
Comment 12 :Ms2ger 2011-06-24 02:33:31 PDT
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.)
Comment 13 Geoff Lankow (:darktrojan) 2011-06-24 21:18:50 PDT
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!)
Comment 14 :Ms2ger 2011-06-25 00:44:18 PDT
No idea
Comment 15 neil@parkwaycc.co.uk 2011-06-25 09:47:19 PDT
Could you have not at least removed all of the in-tree callers before warning about them to other people?
Comment 16 Marek Stępień [:marcoos, inactive] 2011-06-29 14:51:41 PDT
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?
Comment 17 Marek Stępień [:marcoos, inactive] 2011-06-29 14:57:46 PDT
Ignore my last comment, I just read bug 611787 and today's DOM Core draft.
Comment 18 Eric Shepherd [:sheppy] 2011-08-05 12:34:09 PDT
Is my goal documenting this to flag them all as deprecated, basically?
Comment 19 Kris Maglione [:kmag] 2011-08-05 12:39:13 PDT
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.
Comment 20 :Ms2ger 2011-08-05 12:47:50 PDT
They are going away entirely, so something stronger than "deprecated" may be appropriate.
Comment 21 Eric Shepherd [:sheppy] 2011-08-05 12:48:58 PDT
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.
Comment 22 Ioana (away) 2011-09-14 06:24:06 PDT
Created attachment 560140 [details]
test case
Comment 23 Ioana (away) 2011-09-14 06:25:25 PDT
Created attachment 560141 [details]
test case auxiliary file
Comment 24 Ioana (away) 2011-09-14 07:42:08 PDT
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
Comment 25 Eric Shepherd [:sheppy] 2011-09-16 11:29:51 PDT
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?
Comment 26 :Ms2ger 2011-09-17 05:20:23 PDT
Created attachment 560698 [details]
test case
Comment 27 :Ms2ger 2011-09-17 05:25:53 PDT
Looks like quickstubs are interfering.
Comment 28 Eric Shepherd [:sheppy] 2011-09-20 11:38:51 PDT
Documentation updated:

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

Also mentioned on Firefox 7 for developers.
Comment 29 spam.dump.one 2012-02-23 06:01:33 PST
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>
Comment 30 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-02-23 06:04:36 PST
We should file an evangelism bug on jquery.  Ms2ger?

Also means we probably won't be removing these for a while.
Comment 31 spam.dump.one 2012-02-23 06:16:43 PST
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
Comment 32 :Ms2ger 2012-02-23 10:26:42 PST
Can't reproduce either with that snippet; could this be an extension you have installed?

Note You need to log in before you can comment on or make changes to this bug.