Closed Bug 1469307 Opened 2 years ago Closed 2 years ago

Pretty-print [object Attr] in format.pprint

Categories

(Testing :: Marionette, enhancement, P1)

enhancement

Tracking

(firefox62 fixed)

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

Attachments

(1 file)

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…)
Assignee: nobody → ato
Status: NEW → ASSIGNED
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 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 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 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 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 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.
Attachment #8985944 - Flags: review?(hskupin) → review+
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?
Priority: -- → P1
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.
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff262b71ac16
Pretty-print Attr objects in format.pprint. r=whimboo
https://hg.mozilla.org/mozilla-central/rev/ff262b71ac16
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.