Warn in of to-be-removed Attr functions

RESOLVED FIXED in mozilla7

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

(Depends on: 2 bugs, Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla7
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 539172 [details] [diff] [review]
Patch v1
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+

Comment 3

6 years ago
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
Last Resolved: 6 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 → ---
(Assignee)

Comment 8

6 years ago
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."
(Assignee)

Comment 10

6 years ago
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"?
(Assignee)

Comment 12

6 years ago
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
Last Resolved: 6 years ago6 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!)
(Assignee)

Comment 14

6 years ago
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.

Updated

6 years ago
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.
(Assignee)

Comment 20

6 years ago
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.

Comment 22

6 years ago
Created attachment 560140 [details]
test case

Comment 23

6 years ago
Created attachment 560141 [details]
test case auxiliary file

Updated

6 years ago
Attachment #560140 - Attachment is obsolete: true

Updated

6 years ago
Attachment #560141 - Attachment is obsolete: true

Updated

6 years ago
Attachment #560140 - Attachment is obsolete: false

Updated

6 years ago
Attachment #560141 - Attachment is obsolete: false

Comment 24

6 years ago
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?
(Assignee)

Comment 26

6 years ago
Created attachment 560698 [details]
test case
Attachment #560140 - Attachment is obsolete: true
(Assignee)

Comment 27

6 years ago
Looks like quickstubs are interfering.
Documentation updated:

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

Also mentioned on Firefox 7 for developers.
Keywords: dev-doc-needed → dev-doc-complete
Depends on: 690120

Updated

6 years ago
Depends on: 669400

Updated

6 years ago
Depends on: 674437

Updated

6 years ago
Depends on: 679684

Updated

6 years ago
Depends on: 705110

Comment 29

5 years ago
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.

Comment 31

5 years ago
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
(Assignee)

Comment 32

5 years ago
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.