Closed Bug 1341390 Opened 5 years ago Closed 3 years ago

Ensure dom/tests/js and dom/tests/html are covered by automated tests and then delete them


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




Tracking Status
firefox64 --- fixed


(Reporter: jmaher, Assigned: dpino)



(Keywords: good-first-bug)


(1 file)

I cannot find reference in-tree to js/ or html/ being used or mentioned.  Can we get a readme file to indicate why these exist, or just delete these files?
:overholt, can you help determine what these directories/files are used for?  hg history shows the cvs import from 2007 as all changes (other than wide sweeping mass changes)
Flags: needinfo?(overholt)
Blocks: 1339232
Depends on: 1341356
No longer depends on: 1341356
I see no references, either. Comments like Boris' in bug 345521 comment 8 seem to back this up. Let's confirm with Boris.
Flags: needinfo?(overholt) → needinfo?(bzbarsky)
These are manual tests, afaict.

We could try ensuring that they're ported to one of our automated suites when possible, then delete them.
Flags: needinfo?(bzbarsky)
Keywords: good-first-bug
Priority: -- → P5
Summary: dom/tests/js and dom/tests/html do not appear to be used at all → Ensure dom/tests/js and dom/tests/html are covered by automated tests and then delete them

All the tests in `dom/tests/html` and `dom/tests/js` are covered, at least, by web-platform-tests. Both directories can be removed.

The tests check very basic DOM functionality and several Javascript events.


* dom/tests/html/jshandlers.html:

The test doesn't longer work. It breaks returning the following error:

TypeError: l is null; can't access its "onmousedown" property[Learn More]

What the test tries to do is to verify that `onmousedown`, `onmouseup` and `onmousemove` events of an anchor tag work. To do so, it overwrites these events and prints out a message using `dump` [1] (now deprecated).

Covered by: html/browsers/the-window-object/window-properties.https.html and ./html/editing/dnd/events/events-suite-manual.html.

* dom/tests/js/attributes.html:

The tests verifies the following operations related with DOM attributes:

- attribute.getNamedItem == getAttributeNode: true
- attribute BGCOLOR=#ffffff
- changing attribute node value changes attribute value: true
- return value of removeNamedItem is attribute node: true
- removing attribute changes attribute count: true
- changing disembodied attribute value works: true
- removing attribute node removes attribute: true

Covered by: `dom/nodes/attributes.html`.

The test relies on `dump`.

* dom/tests/js/class.html:

Tests DOM node className field.

Covered by: `dom/nodes/Node-properties.html`.

* dom/tests/js/clone.html:

Tests DOM node `cloneNode()` method.

Covered by: `dom/nodes/Node-cloneNode.html`.

* dom/tests/js/docfrag.html:

Tests several DOM operations: `createDocumentFragment()`, `replaceChild()`, `createTextNode()`, `appendChild()`

Covered by: `dom/nodes/Node-replaceChild.html`.

* dom/tests/js/id.html:

Tests `getElementById()`.

Covered by: `dom/nodes/Document-getElementById.html`.

* dom/tests/js/lists.html:

Iterates over document.images, document.links and document.anchors printing out some of the nodes attribute values via `getDOMAttribute()`.

The test returns the following error:

document.images[x].getDOMAttribute is not a function[Learn More]

`getDOMAttribute` is no longer part of the DOM API. Apparently it does the same as `node.getAttribute(name)`.

The test relies on `dump`.

Covered by: `dom/nodes/attributes.html`.

* dom/tests/js/style1.html:

Defines several CSS styles and perform several style changes in some document tags via `style` attribute. These changes
are done by calling a function (changeColor) every 40 ms. The function doesn't fully work printing out the following
error in the console:

TypeError: h is undefined; can't access its "style" property[Learn More]
TypeError: can't access property "style" of undefined[Learn More]

Covered by: `css/cssom/cssstyledeclaration-csstext.html` and `css/css-transitions/events-007.html`.

* dom/tests/js/style2.html:

Similar to `style1.html`.

The test prints out the following error in the console:

TypeError: can't access property "style" of undefined[Learn More] style1.html:103:3
TypeError: document.documentElement.childNodes[1].childNodes[1] is undefined; can't access its "childNodes" property[Learn More]

Covered by: `css/cssom-view/scrolling-quirks-vs-nonquirks.html`.

* dom/tests/js/write2.html:

Tests `document.write()` and `document.close()`.

Covered by: `./html/webappapis/dynamic-markup-insertion/document-write/033.html` and `./html/webappapis/dynamic-markup-insertion/document-write/50.html`.

* dom/tests/js/write.html:

Tests `document.writeln()`.

Covered by: `./html/webappapis/dynamic-markup-insertion/document-write/033.html`.

There are other JavaScript tests in dom/html:

- DumpHTML.js: iterates an HTML document and prints out in console.
- DumpTree.js: Traverse a tree of DOM nodes.
- HTMLString.js: Starting from document.documentElement, prints out tree. 
- simple.js: Access several DOM node and attribute fields such as: node.hasChildNodes, node.tagName, node.nextSibling, or attribute.value.
- ssheets.js: Iterates over a stylesheet and access several stylesheet properties.
- timer.js: Uses setTimeout and setInterval.

I didn't exhaustively looked into web-platform-tests for these JavaScript tests, but likely all this is covered since they are very basic operations.

Attachment #9017120 - Flags: review?(bzbarsky)
> Covered by: `dom/nodes/Node-properties.html`.

What dom/tests/js/class.html is really testing is that the class change triggers restyling and relayout.  The  dom/nodes/Node-properties.html wpt doesn't test that, but I'm pretty sure other wpts do.

> Tests `getElementById()`.

No, tests that changing the id does restyling and relayout.  Again probably covered by wpts, but not by `dom/nodes/Document-getElementById.html
Comment on attachment 9017120 [details] [diff] [review]

r=me.   Thank you for the patch and the analysis!  It's very much appreciated.
Attachment #9017120 - Flags: review?(bzbarsky) → review+
Assignee: nobody → dpino
Pushed by
Remove unused tests dom/tests/html and dom/tests/js.  r=bzbarsky
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.