Closed
Bug 1469307
Opened 6 years ago
Closed 6 years ago
Pretty-print [object Attr] in format.pprint
Categories
(Remote Protocol :: Marionette, enhancement, P1)
Remote Protocol
Marionette
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 | ||
Updated•6 years ago
|
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
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.
Assignee | ||
Comment 3•6 years ago
|
||
mozreview-review-reply |
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•6 years ago
|
||
mozreview-review-reply |
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.
Assignee | ||
Comment 5•6 years ago
|
||
mozreview-review-reply |
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•6 years ago
|
||
mozreview-review-reply |
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•6 years ago
|
||
mozreview-review |
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+
Assignee | ||
Comment 8•6 years ago
|
||
mozreview-review-reply |
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?
Assignee | ||
Updated•6 years ago
|
Priority: -- → P1
Comment 9•6 years ago
|
||
mozreview-review-reply |
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•6 years ago
|
||
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff262b71ac16
Pretty-print Attr objects in format.pprint. r=whimboo
Comment 11•6 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•