1469307 - Pretty-print [object Attr] in format.pprint

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement, P1)

Description

[Andreas Tolfsen ❲:ato❳]

To ease with debugging, format.pprint should support pretty printing
Attr objects, identified by [object Attr].

(Although ideally we should borrow the pretty-printing formatter
from devtools…)

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1469307 - Pretty-print Attr objects in format.pprint.

Attr objects are returned from Node.getAttribute() and this provides
a readable string like the one offered in devtools for them.

Review commit: https://reviewboard.mozilla.org/r/251422/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/251422/

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257828

::: testing/marionette/format.js:68
(Diff revision 1)
>      return `[${proto.substring(1, proto.length - 1)} ${win.location}]`;
>    }
>  
> +  function prettyAttr(obj) {
> +    return `[object Attr ${obj.name}="${obj.value}"]`;
> +  }

Is that in some way related to the iteration through attributes in `prettyElement()`? Or when is this bring called specifically? Maybe give an example, and add some documentation to the code to make it clear.

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257828

> Is that in some way related to the iteration through attributes in `prettyElement()`? Or when is this bring called specifically? Maybe give an example, and add some documentation to the code to make it clear.

I’m not sure I understand entirely what you mean.  This prints
individual Attr objects, whereas prettyElement selects and prints
some attributes from an element we think are important.  You can
get at Attr objects by iterating over NamedNodeMap.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257828

> I’m not sure I understand entirely what you mean.  This prints
> individual Attr objects, whereas prettyElement selects and prints
> some attributes from an element we think are important.  You can
> get at Attr objects by iterating over NamedNodeMap.

How would a specific usage look like? I never heard about Attr objects yet. Having some docs in the code might also help.

Comment 5

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257828

> How would a specific usage look like? I never heard about Attr objects yet. Having some docs in the code might also help.

See https://dom.spec.whatwg.org/#interface-attr.

NamedNodeMap is a collection of Attr’s returned from Node.attributes.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257828

> See https://dom.spec.whatwg.org/#interface-attr.
> 
> NamedNodeMap is a collection of Attr’s returned from Node.attributes.

The example on MDN was actually helpful. So it looks fine.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257968

::: testing/marionette/format.js:43
(Diff revision 1)
>      if (val && val.nodeType === 1) {
>        return prettyElement(val);
>      } else if (["[object Window]", "[object ChromeWindow]"].includes(proto)) {
>        return prettyWindowGlobal(val);
> +    } else if (proto == "[object Attr]") {
> +      return prettyAttr(val);

Please add a unit test for this addition.

Comment 8

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257968

> Please add a unit test for this addition.

I think I’ve explained before that I haven’t found a way to test
protos.  Object.prototype.toString.call always returns the underlying
prototype and I’m not sure there is a way to change it so I can
mock out an Attr object?

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 8985944 [details]
Bug 1469307 - Pretty-print Attr objects in format.pprint.

https://reviewboard.mozilla.org/r/251422/#review257968

> I think I’ve explained before that I haven’t found a way to test
> protos.  Object.prototype.toString.call always returns the underlying
> prototype and I’m not sure there is a way to change it so I can
> mock out an Attr object?

In that case just skip it. I didn't remember that.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff262b71ac16
Pretty-print Attr objects in format.pprint. r=whimboo

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ff262b71ac16