Closed Bug 1345119 Opened 7 years ago Closed 7 years ago

Display offset parent used for positioning in box model

Categories

(DevTools :: Inspector, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 55

People

(Reporter: lockhart, Assigned: lockhart)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qa-triaged])

Attachments

(5 files, 3 obsolete files)

59 bytes, text/x-review-board-request
pbro
: review+
Details
59 bytes, text/x-review-board-request
pbro
: review+
Details
59 bytes, text/x-review-board-request
pbro
: review+
Details
59 bytes, text/x-review-board-request
Details
59 bytes, text/x-review-board-request
Details
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_3) AppleWebKit/602.4.8 (KHTML, like Gecko) Version/10.0.3 Safari/602.4.8
Blocks: 1150496
Component: Untriaged → Developer Tools: Inspector
Assignee: nobody → lockhart
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P3
Comment on attachment 8847301 [details]
Bug 1345119 - Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel.

This is just an initial implementation of the offset parent.  I am mostly looking for feedback regarding my implementation of the actor/spec/server section - the UI code is there to verify that it was working.

I also had a couple questions about the display format as it is in https://projects.invisionapp.com/share/W287NPLAQ#/screens/179720590_Better_Box_Model.

There, the offset parent is displayed as the class selector, which is why in this patch I default to using the full CSS path.  How do we want it displayed? Do we want the minimal representation, or a full path, or something in between? Displaying just the class seems it would lead to ambiguity.

Also, I was wondering how to source the (what I assume is) SVG for the icon used to grab the parent context

Thanks
Attachment #8847301 - Flags: feedback?(pbrosset)
Comment on attachment 8847301 [details]
Bug 1345119 - Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel.

Thanks for the patch. That's a good start. I have a comment and a reply for you below:

- You have introduced a new NodeActor method: getOffsetParent(), which is great, but you need to keep in mind that DevTools has the ability to connect to any debugging target: a page running in the same browser, or a page running in an other (possibly older) Firefox browser running elsewhere, or a page running on a remote Android device. This means that the client-side (the UI) code can't make assumptions about what exists or not on the actor side. So calling node.getOffsetParent() directly may fail if you're connected to an older Firefox.
You need to first detect if the method exists, and if not just hide the feature altogether. You can do this with actorHasMethod. There are several usage examples here: http://searchfox.org/mozilla-central/search?q=actorHasMethod&path=devtools

- Displaying the DOM Node in the box model: the mockups show a short preview of the node, this is similar to what we use in many places of devtools. Like, when you output a node in the console, you get some sort of preview for it. Same in the debugger variables sidebar, or in the animation inspector, etc. There are many places where we need to display some preview form for DOM nodes. The way we do this is we use shared components. We unfortunately don't have just one of them, but we're converging to using the ElementNode Rep.
This is a React component that already can consume a NodeFront a preview it in various ways. I think the best thing you can do is reuse some of the code that was written recently for grid containers: http://searchfox.org/mozilla-central/rev/ca7015fa45b30b29176fbaa70ba0a36fe9263c38/devtools/client/inspector/grids/components/GridItem.js#133-141
Attachment #8847301 - Flags: feedback?(pbrosset)
Blocks: 1347964
Patrick, those patches should be good for review.  One thing I noticed was that the ElementNode Rep can sometimes be a little long, and it would overlap with the rest of the BoxModel.  I didn't think it was a big enough issue to hold off on starting the review, but if you have any suggestions on how to fix that I would appreciate it.
Flags: needinfo?(pbrosset)
Comment on attachment 8847301 [details]
Bug 1345119 - Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel.

https://reviewboard.mozilla.org/r/120310/#review123820

Thanks for the cleanup. I think we can get rid of a function though.

::: devtools/client/inspector/inspector.js:1954
(Diff revision 2)
> +  /**
> +   * Set the inspector selection.
> +   *
> +   * @param {NodeFront} nodeFront
> +   *        The NodeFront corresponding to the new selection.
> +   */
> +  setSelectedNode(nodeFront, reason) {
> +    this.selection.setNodeFront(nodeFront, reason);
> +  },

I'm not sure we should be adding this new function here.
We use `inspector.selection.setNodeFront` in many places in the code already, so if we introduce a new method in the inspector object, we should make all of these places use `inspector.setSelectedNode` instead.

But I'm not sure it's really worth it. 
`inspector.selection` is a well known public API we use already, let's just stick with this one and remove this new function.
Attachment #8847301 - Flags: review?(pbrosset) → review+
Comment on attachment 8848691 [details]
Bug 1345119 - Part 2: Server side for retrieving offset parent of DOM node.

https://reviewboard.mozilla.org/r/121588/#review123824

I suggest moving the new actor method to the `WalkerActor` instead. This way, it's more consistent with the rest of the code. The Walker is where we get nodes from. We don't yet have any method on the `NodeActor` that returns other nodes, wherease this is essentially what the Walker is for.
Of course, doing this will require a small signature change:
`getOffsetParent(node)` instead of `getOffsetParent()`.

::: devtools/server/actors/inspector.js:764
(Diff revision 1)
> +    if (!offsetParent ||
> +      (offsetParent && CssLogic.getComputedStyle(offsetParent).position === "static")) {

The condition can be simplified a little bit, after the `||` part we now `offsetParent` is not falsy, so no need to repeat it there:

```
if (!offsetParent || CssLogic.getComputedStyle(offsetParent).position === "static") {
```

::: devtools/shared/fronts/inspector.js:131
(Diff revision 1)
>      // Shallow copy of the form.  We could just store a reference, but
>      // eventually we'll want to update some of the data.
>      this._form = object.merge(form);
>      this._form.attrs = this._form.attrs ? this._form.attrs.slice() : [];
>  
> -    if (form.parent) {
> +    if (form.parent && ctx.marshallPool().ensureParentFront) {

This change is not needed if you move the method to the walker actor instead.

::: devtools/shared/specs/node.js:13
(Diff revision 1)
>    RetVal,
>    generateActorSpec,
>    types
>  } = require("devtools/shared/protocol.js");
>  
> +types.addActorType("domnode");

You will also be able to remove this.
Attachment #8848691 - Flags: review?(pbrosset)
Comment on attachment 8848691 [details]
Bug 1345119 - Part 2: Server side for retrieving offset parent of DOM node.

https://reviewboard.mozilla.org/r/121588/#review123828

I forgot to mention: we also need a new test for the method, so we can avoid regressions in the future, and also easily encode edge cases.
There are many walker actor tests in these files already: `\devtools\server\tests\mochitest\test_inspector-*.html`, so I think it should be relatively easy to add a new one to test the new method.
Comment on attachment 8848692 [details]
Bug 1345119 - Part 3: Display offset parent of absolutely positioned node in box model.

https://reviewboard.mozilla.org/r/121590/#review123830

This looks mostly good to me. I just made a suggestion to use the MODE.TINY so the node preview is shorter.
Can you ask jdescottes for the final review on this please? I'd feel more comfortable if he did the review, since he knows the ElementNode Rep, he's already used it in other places.

::: devtools/client/inspector/boxmodel/components/BoxModelMain.js:206
(Diff revision 1)
> +            className: "boxmodel-offset-parent",
> +            "data-box": "offset-parent",
> +          },
> +          Rep(
> +            {
> +              defaultRep: offsetParent,

Please also add the `mode: MODE.TINY,` property in this config object, so that the node appears in its tiny form: `tagName#id.class`. This is how we display DOM nodes in other places too.
Attachment #8848692 - Flags: review?(pbrosset)
Attachment #8847301 - Attachment is obsolete: true
Attachment #8848691 - Attachment is obsolete: true
Attachment #8848691 - Flags: review?(pbrosset)
Attachment #8848692 - Attachment is obsolete: true
Attachment #8848692 - Flags: review?(jdescottes)
Attachment #8849790 - Flags: review?(pbrosset) → review+
Apologies - accidentally marked the other patches as obsolete while amending Part 4.  Should be okay now.

I still have tests that I would like to write for the BoxModel code, that will come in a Part 5, but these 4 patches should fix the comments above.
Comment on attachment 8849792 [details]
Bug 1345119 - Part 3: Display offset parent of absolutely positioned node in box model.

https://reviewboard.mozilla.org/r/122546/#review124730

::: devtools/client/inspector/boxmodel/box-model.js:171
(Diff revision 1)
>        let isPositionEditable = yield this.inspector.pageStyle.isPositionEditable(node);
>        layout = Object.assign({}, layout, {
>          isPositionEditable,
>        });
>  
> +      if (yield this.inspector.target.actorHasMethod("domwalker", "getOffsetParent")) {

I would assume an variable actorCanGetOffSetParent for yield this.inspector.target.actorHasMethod("domwalker", "getOffsetParent")

::: devtools/client/inspector/boxmodel/components/BoxModel.js:28
(Diff revision 1)
>      onHideBoxModelHighlighter: PropTypes.func.isRequired,
>      onShowBoxModelEditor: PropTypes.func.isRequired,
>      onShowBoxModelHighlighter: PropTypes.func.isRequired,
> +    onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
>      onToggleGeometryEditor: PropTypes.func.isRequired,
> +    setSelectedNode: PropTypes.func.isRequired,

Move this below the boxModel

::: devtools/client/inspector/boxmodel/components/BoxModel.js:42
(Diff revision 1)
>        onHideBoxModelHighlighter,
>        onShowBoxModelEditor,
>        onShowBoxModelHighlighter,
> +      onShowBoxModelHighlighterForNode,
>        onToggleGeometryEditor,
> +      setSelectedNode,

Move this above showBoxModelProperties

::: devtools/client/inspector/boxmodel/components/BoxModel.js:55
(Diff revision 1)
>          boxModel,
>          onHideBoxModelHighlighter,
>          onShowBoxModelEditor,
>          onShowBoxModelHighlighter,
> +        onShowBoxModelHighlighterForNode,
> +        setSelectedNode,

Move this below boxModel. I usually have all the handlers (onX functions) at the bottom.

::: devtools/client/inspector/boxmodel/components/BoxModelApp.js:34
(Diff revision 1)
>      onHideBoxModelHighlighter: PropTypes.func.isRequired,
>      onShowBoxModelEditor: PropTypes.func.isRequired,
>      onShowBoxModelHighlighter: PropTypes.func.isRequired,
> +    onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
>      onToggleGeometryEditor: PropTypes.func.isRequired,
> +    setSelectedNode: PropTypes.func.isRequired,

Move this below boxModel

::: devtools/client/inspector/boxmodel/components/BoxModelMain.js:35
(Diff revision 1)
>      boxModel: PropTypes.shape(Types.boxModel).isRequired,
>      onHideBoxModelHighlighter: PropTypes.func.isRequired,
>      onShowBoxModelEditor: PropTypes.func.isRequired,
>      onShowBoxModelHighlighter: PropTypes.func.isRequired,
> +    onShowBoxModelHighlighterForNode: PropTypes.func.isRequired,
> +    setSelectedNode: PropTypes.func.isRequired,

Move this below boxModel

::: devtools/client/inspector/boxmodel/components/BoxModelMain.js:135
(Diff revision 1)
> +   *
> +   * @params  {NodeFront} nodeFront
> +   *          The NodeFront for which we want to create a grip-like object.
> +   * @returns {Object} a grip-like object that can be used with Reps.
> +   */
> +  translateNodeFrontToGrip(nodeFront) {

Move this function above onHighlightMouseOver

::: devtools/client/inspector/boxmodel/components/BoxModelMain.js:163
(Diff revision 1)
> +
>    render() {
> -    let { boxModel, onShowBoxModelEditor } = this.props;
> -    let { layout } = boxModel;
> +    let {
> +        boxModel,
> +        onShowBoxModelEditor,
> +        setSelectedNode,

Move this below boxModel

::: devtools/client/inspector/computed/computed.js:642
(Diff revision 1)
>          onHideBoxModelHighlighter,
>          onShowBoxModelEditor,
>          onShowBoxModelHighlighter,
> +        onShowBoxModelHighlighterForNode,
>          onToggleGeometryEditor,
> +        setSelectedNode,

Move this above showBoxModelProperties
Attachment #8849792 - Flags: review?(jdescottes)
Comment on attachment 8849791 [details]
Bug 1345119 - Part 2: Server side for retrieving offset parent of DOM node.

https://reviewboard.mozilla.org/r/122544/#review124892

Great! Thanks. 
Just a few clean ups to avoid adding unrelated changes to the hg history. Otherwise ready to go.

::: devtools/server/actors/inspector.js:753
(Diff revision 1)
>        fillStyle: fillStyle
>      };
>      let { dataURL, size } = getFontPreviewData(font, doc, options);
>  
>      return { data: LongStringActor(this.conn, dataURL), size: size };
> -  }
> +  },

nit: since you didn't touch this part of the code after all, better turn it back into what it was exactly, so the hg history isn't polluted i.e.: remove the extra comma here.

::: devtools/server/actors/inspector.js:2911
(Diff revision 1)
>      events.emit(this, "color-picked", color);
>    },
>  
>    _onColorPickCanceled: function () {
>      events.emit(this, "color-pick-canceled");
> -  }
> +  },

nit: since you didn't touch this part of the code after all, better turn it back into what it was exactly, so the hg history isn't polluted i.e.: remove the extra comma here.

::: devtools/shared/specs/node.js:69
(Diff revision 1)
>        response: {}
>      },
>      getFontFamilyDataURL: {
>        request: {font: Arg(0, "string"), fillStyle: Arg(1, "nullable:string")},
>        response: RetVal("imageData")
> +    },

nit: since you didn't touch this part of the code after all, better turn it back into what it was exactly, so the hg history isn't polluted i.e.: remove the extra comma here.
Attachment #8849791 - Flags: review?(pbrosset) → review+
Comment on attachment 8849788 [details]
Bug 1345119 - Part 4: Test test_inspector_getOffsetParent for new actor methods.

https://reviewboard.mozilla.org/r/122540/#review124896

Really good test, thank you!
This made me realize I was actually not sure about the method's return values though. See my question below.

::: commit-message-e2ec4:1
(Diff revision 2)
> +Bug 1345119 - Part 4: Tests for new actor methods. r?pbro

Maybe add the name of the method in the commit message, to make it more self-explanatory.

::: devtools/server/tests/mochitest/test_inspector_getOffsetParent.html:25
(Diff revision 2)
> +
> +var gWalker;
> +
> +addTest(function setup() {
> +  let url = document.getElementById("inspectorContent").href;
> +  info("Got URL");

I don't think this `info` serves much purpose. No need surcharging log files with things we're not really going to read when debugging tests.

::: devtools/server/tests/mochitest/test_inspector_getOffsetParent.html:27
(Diff revision 2)
> +
> +addTest(function setup() {
> +  let url = document.getElementById("inspectorContent").href;
> +  info("Got URL");
> +  attachURL(url, function(err, client, tab, doc) {
> +    info("attachURL complete");

Same here. I know you probably copy/pasted code from another existing test, but since this one is new, let's try and make it as nice as possible.

::: devtools/server/tests/mochitest/test_inspector_getOffsetParent.html:32
(Diff revision 2)
> +    info("attachURL complete");
> +    let {InspectorFront} = require("devtools/shared/fronts/inspector");
> +    let inspector = InspectorFront(client, tab);
> +
> +    promiseDone(inspector.getWalker().then(walker => {
> +      info("Got Walker");

Same.

::: devtools/server/tests/mochitest/test_inspector_getOffsetParent.html:56
(Diff revision 2)
> +addTest(function() {
> +  info("Try to get the offset parent for a node that is absolutely positioned outside a relative node");
> +  gWalker.querySelector(gWalker.rootNode, "#no_parent").then(node => {
> +    return gWalker.getOffsetParent(node);
> +  }).then(offsetParent => {
> +    ok(!offsetParent, "The node does not have an offset parent");

This actually makes me think about the feature. Why would we want positioned elements to ever have a null offsetParent?
This doesn't happen in the page. If a positioned element is not a descendant of a positioned ancestor, then its offsetParent is <body>, right?
Shouldn't the method actually return body in these cases.
Attachment #8849788 - Flags: review?(pbrosset)
Comment on attachment 8849790 [details]
Bug 1345119 - Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel.

https://reviewboard.mozilla.org/r/122542/#review124900

Thanks.
Attachment #8849790 - Flags: review+
Patrick, I took what you said and changed getOffsetParent to always return the offset parent when it exists.  In the BoxModel, we now just decide to display it only when the selected node has "position: absolute".

I updated the tests to reflect that change, and added another for good measure.

I think this makes it more in line with what is expected if someone else were to call getOffsetParent.
Comment on attachment 8849792 [details]
Bug 1345119 - Part 3: Display offset parent of absolutely positioned node in box model.

Setting myself as the reviewer since jdescottes is away this week
Attachment #8849792 - Flags: review?(jdescottes) → review?(gl)
Comment on attachment 8849788 [details]
Bug 1345119 - Part 4: Test test_inspector_getOffsetParent for new actor methods.

https://reviewboard.mozilla.org/r/122540/#review125258

Awesome!
Attachment #8849788 - Flags: review?(pbrosset) → review+
Clearing old NI? flag.
Flags: needinfo?(pbrosset)
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca6b6c3fe79f
Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c1ee3920ea3
Part 2: Server side for retrieving offset parent of DOM node. r=pbro
Comment on attachment 8849792 [details]
Bug 1345119 - Part 3: Display offset parent of absolutely positioned node in box model.

https://reviewboard.mozilla.org/r/122546/#review125594
Attachment #8849792 - Flags: review?(gl) → review+
Comment on attachment 8849792 [details]
Bug 1345119 - Part 3: Display offset parent of absolutely positioned node in box model.

https://reviewboard.mozilla.org/r/122546/#review125596
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/14ff5693848f
Part 3: Display offset parent of absolutely positioned node in box model. r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/c67e5d94d6c3
Part 4: Test test_inspector_getOffsetParent for new actor methods. r=pbro
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7145d2ae3fab
Part 5: Fix eslint errors in unit tests. r=me
Backed out for failing various devtools tests, e.g. devtools/client/inspector/test/browser_inspector_textbox-menu.js:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c8379919d781f0aeb8eadf124f47ed89541731ed
https://hg.mozilla.org/integration/mozilla-inbound/rev/160438b0f0f8d5e463a7f2a0bbb7f3412f7e5f42
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cb92f5cc3bc091f05b494cae322d85066ed1df6
https://hg.mozilla.org/integration/mozilla-inbound/rev/bdc52a612f3e16b1887113879a7f54f06ccd0c10
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d8420156bc280ee05562d6bcf9093ffa2360b0a

Push with failures (part 3+4), please check all the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c67e5d94d6c3f8eeb431ff964c73471cb86016cf&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log devtools/client/inspector/test/browser_inspector_textbox-menu.js: https://treeherder.mozilla.org/logviewer.html#?job_id=86033996&repo=mozilla-inbound

[task 2017-03-23T20:41:14.930501Z] 20:41:14     INFO - Got event: 'ruleview-changed' on [object Object].
[task 2017-03-23T20:41:14.931300Z] 20:41:14     INFO - TEST-PASS | devtools/client/inspector/test/browser_inspector_textbox-menu.js | The menu is closed - 
[task 2017-03-23T20:41:14.932539Z] 20:41:14     INFO - Simulating context click on the textbox and expecting the menu to open
[task 2017-03-23T20:41:14.933563Z] 20:41:14     INFO - Waiting for event: 'popupshown' on [object XULElement].
[task 2017-03-23T20:41:14.934638Z] 20:41:14     INFO - Got event: 'popupshown' on [object XULElement].
[task 2017-03-23T20:41:14.935788Z] 20:41:14     INFO - TEST-PASS | devtools/client/inspector/test/browser_inspector_textbox-menu.js | The menu is now visible - 
[task 2017-03-23T20:41:14.936482Z] 20:41:14     INFO - Closing the menu
[task 2017-03-23T20:41:14.937037Z] 20:41:14     INFO - Waiting for event: 'popuphidden' on [object XULElement].
[task 2017-03-23T20:41:14.937605Z] 20:41:14     INFO - Got event: 'popuphidden' on [object XULElement].
[task 2017-03-23T20:41:14.938044Z] 20:41:14     INFO - TEST-PASS | devtools/client/inspector/test/browser_inspector_textbox-menu.js | The menu is closed again - 
[task 2017-03-23T20:41:14.938631Z] 20:41:14     INFO - Switching to the computed-view
[task 2017-03-23T20:41:14.939423Z] 20:41:14     INFO - Buffered messages logged at 20:40:09
[task 2017-03-23T20:41:14.940607Z] 20:41:14     INFO - Console message: [JavaScript Error: "1490301609207	Browser.Experiments.Experiments	ERROR	Experiments #0::httpGetRequest::onLoad() - Request to http://127.0.0.1:8888/experiments-dummy/manifest returned status 404" {file: "resource://gre/modules/Log.jsm" line: 748}]
[task 2017-03-23T20:41:14.941405Z] 20:41:14     INFO - App_append@resource://gre/modules/Log.jsm:748:9
[task 2017-03-23T20:41:14.941767Z] 20:41:14     INFO - log@resource://gre/modules/Log.jsm:386:7
[task 2017-03-23T20:41:14.942429Z] 20:41:14     INFO - getLoggerWithMessagePrefix/proxy.log@resource://gre/modules/Log.jsm:501:44
[task 2017-03-23T20:41:14.943017Z] 20:41:14     INFO - Experiments.Experiments/this._log.log@resource://app/modules/experiments/Experiments.jsm:321:5
[task 2017-03-23T20:41:14.943392Z] 20:41:14     INFO - error@resource://gre/modules/Log.jsm:394:5
[task 2017-03-23T20:41:14.944156Z] 20:41:14     INFO - _httpGetRequest/xhr.onload@resource://app/modules/experiments/Experiments.jsm:952:9
[task 2017-03-23T20:41:14.944747Z] 20:41:14     INFO - EventHandlerNonNull*_httpGetRequest@resource://app/modules/experiments/Experiments.jsm:950:5
[task 2017-03-23T20:41:14.945402Z] 20:41:14     INFO - _loadManifest@resource://app/modules/experiments/Experiments.jsm:814:32
[task 2017-03-23T20:41:14.946018Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:319:42
[task 2017-03-23T20:41:14.946609Z] 20:41:14     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-03-23T20:41:14.947269Z] 20:41:14     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-03-23T20:41:14.947892Z] 20:41:14     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-03-23T20:41:14.948509Z] 20:41:14     INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16
[task 2017-03-23T20:41:14.949173Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15
[task 2017-03-23T20:41:14.950038Z] 20:41:14     INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:401:7
[task 2017-03-23T20:41:14.950654Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15
[task 2017-03-23T20:41:14.951247Z] 20:41:14     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-03-23T20:41:14.951931Z] 20:41:14     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-03-23T20:41:14.952720Z] 20:41:14     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-03-23T20:41:14.953265Z] 20:41:14     INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16
[task 2017-03-23T20:41:14.953831Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15
[task 2017-03-23T20:41:14.954405Z] 20:41:14     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-03-23T20:41:14.954932Z] 20:41:14     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-03-23T20:41:14.955479Z] 20:41:14     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-03-23T20:41:14.955868Z] 20:41:14     INFO - _run@resource://app/modules/experiments/Experiments.jsm:759:24
[task 2017-03-23T20:41:14.956499Z] 20:41:14     INFO - updateManifest@resource://app/modules/experiments/Experiments.jsm:847:12
[task 2017-03-23T20:41:14.957023Z] 20:41:14     INFO - notify@resource://app/components/ExperimentsService.js:66:7
[task 2017-03-23T20:41:14.957739Z] 20:41:14     INFO - TM_notify/<@resource://gre/components/nsUpdateTimerManager.js:218:11
[task 2017-03-23T20:41:14.958288Z] 20:41:14     INFO - TM_notify@resource://gre/components/nsUpdateTimerManager.js:263:7
[task 2017-03-23T20:41:14.958797Z] 20:41:14     INFO - 
[task 2017-03-23T20:41:14.959736Z] 20:41:14     INFO - Console message: [JavaScript Error: "1490301609208	Browser.Experiments.Experiments	ERROR	Experiments #0::_loadManifest - failure to fetch/parse manifest (continuing anyway): Error: Experiments - XHR status for http://127.0.0.1:8888/experiments-dummy/manifest is 404" {file: "resource://gre/modules/Log.jsm" line: 748}]
[task 2017-03-23T20:41:14.960169Z] 20:41:14     INFO - App_append@resource://gre/modules/Log.jsm:748:9
[task 2017-03-23T20:41:14.961163Z] 20:41:14     INFO - log@resource://gre/modules/Log.jsm:386:7
[task 2017-03-23T20:41:14.961766Z] 20:41:14     INFO - getLoggerWithMessagePrefix/proxy.log@resource://gre/modules/Log.jsm:501:44
[task 2017-03-23T20:41:14.962355Z] 20:41:14     INFO - Experiments.Experiments/this._log.log@resource://app/modules/experiments/Experiments.jsm:321:5
[task 2017-03-23T20:41:14.962877Z] 20:41:14     INFO - error@resource://gre/modules/Log.jsm:394:5
[task 2017-03-23T20:41:14.963595Z] 20:41:14     INFO - _loadManifest@resource://app/modules/experiments/Experiments.jsm:824:7
[task 2017-03-23T20:41:14.964126Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:320:42
[task 2017-03-23T20:41:14.964736Z] 20:41:14     INFO - process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:925:21
[task 2017-03-23T20:41:14.965269Z] 20:41:14     INFO - walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7
[task 2017-03-23T20:41:14.965829Z] 20:41:14     INFO - Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11
[task 2017-03-23T20:41:14.966220Z] 20:41:14     INFO - schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7
[task 2017-03-23T20:41:14.966790Z] 20:41:14     INFO - completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
[task 2017-03-23T20:41:14.967841Z] 20:41:14     INFO - _httpGetRequest/xhr.onload@resource://app/modules/experiments/Experiments.jsm:953:9
[task 2017-03-23T20:41:14.968489Z] 20:41:14     INFO - EventHandlerNonNull*_httpGetRequest@resource://app/modules/experiments/Experiments.jsm:950:5
[task 2017-03-23T20:41:14.969166Z] 20:41:14     INFO - _loadManifest@resource://app/modules/experiments/Experiments.jsm:814:32
[task 2017-03-23T20:41:14.969899Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:319:42
[task 2017-03-23T20:41:14.970492Z] 20:41:14     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-03-23T20:41:14.971272Z] 20:41:14     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-03-23T20:41:14.971940Z] 20:41:14     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-03-23T20:41:14.972693Z] 20:41:14     INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16
[task 2017-03-23T20:41:14.973250Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15
[task 2017-03-23T20:41:14.973925Z] 20:41:14     INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:401:7
[task 2017-03-23T20:41:14.974526Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15
[task 2017-03-23T20:41:14.975300Z] 20:41:14     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-03-23T20:41:14.975976Z] 20:41:14     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-03-23T20:41:14.976566Z] 20:41:14     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-03-23T20:41:14.977172Z] 20:41:14     INFO - TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:389:16
[task 2017-03-23T20:41:14.977772Z] 20:41:14     INFO - TaskImpl_run@resource://gre/modules/Task.jsm:327:15
[task 2017-03-23T20:41:14.978381Z] 20:41:14     INFO - TaskImpl@resource://gre/modules/Task.jsm:277:3
[task 2017-03-23T20:41:14.978947Z] 20:41:14     INFO - asyncFunction@resource://gre/modules/Task.jsm:252:14
[task 2017-03-23T20:41:14.979320Z] 20:41:14     INFO - Task_spawn@resource://gre/modules/Task.jsm:166:12
[task 2017-03-23T20:41:14.979894Z] 20:41:14     INFO - _run@resource://app/modules/experiments/Experiments.jsm:759:24
[task 2017-03-23T20:41:14.980698Z] 20:41:14     INFO - updateManifest@resource://app/modules/experiments/Experiments.jsm:847:12
[task 2017-03-23T20:41:14.981264Z] 20:41:14     INFO - notify@resource://app/components/ExperimentsService.js:66:7
[task 2017-03-23T20:41:14.982027Z] 20:41:14     INFO - TM_notify/<@resource://gre/components/nsUpdateTimerManager.js:218:11
[task 2017-03-23T20:41:14.982654Z] 20:41:14     INFO - TM_notify@resource://gre/components/nsUpdateTimerManager.js:263:7
[task 2017-03-23T20:41:14.983235Z] 20:41:14     INFO - 
[task 2017-03-23T20:41:14.983934Z] 20:41:14     INFO - Buffered messages finished
[task 2017-03-23T20:41:14.984897Z] 20:41:14     INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/test/browser_inspector_textbox-menu.js | Test timed out - 
[task 2017-03-23T20:41:14.985522Z] 20:41:14     INFO - Removing tab.
[task 2017-03-23T20:41:14.986259Z] 20:41:14     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2017-03-23T20:41:14.987018Z] 20:41:14     INFO - Got event: 'TabClose' on [object XULElement].
[task 2017-03-23T20:41:14.987724Z] 20:41:14     INFO - Tab removed and finished closing
Flags: needinfo?(lockhart)
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba020c15393d
Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c6f67f420d0
Part 2: Server side for retrieving offset parent of DOM node. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a01debcc43c
Part 3: Display offset parent of absolutely positioned node in box model. r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e4817e5b6d4
Part 4: Test test_inspector_getOffsetParent for new actor methods. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9be65ad95f6
Part 5: Fix eslint errors in unit tests. r=me
Flags: needinfo?(lockhart)
Attachment #8849792 - Flags: review?(jdescottes)
Attachment #8849792 - Flags: review+
Comment on attachment 8851249 [details]
Bug 1345119 - Part 5: Test browser_boxmodel_offsetparent.js to test front end box model.

https://reviewboard.mozilla.org/r/123602/#review126106
Attachment #8851249 - Flags: review?(gl) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e84425b5ccef
Part 1: Move functions out of grid-inspector into inspector for sharing with boxmodel. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/03e81669e18c
Part 2: Server side for retrieving offset parent of DOM node. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/130016ad0425
Part 3: Display offset parent of absolutely positioned node in box model. r=gl
https://hg.mozilla.org/integration/mozilla-inbound/rev/37dac6a2af16
Part 4: Test test_inspector_getOffsetParent for new actor methods. r=pbro
https://hg.mozilla.org/integration/mozilla-inbound/rev/23d64298e8c6
Part 5: Test browser_boxmodel_offsetparent.js to test front end box model. r=gl
I think this bug deserves qa verification. 
Gabriel, could you summarize the feature and what would be interesting to test here?

Thanks!
Flags: qe-verify+
Flags: needinfo?(gl)
Flags: needinfo?(gl)
Product: Firefox → DevTools
Hello Gabriel!
This issue has resurfaced in our queries. Can you please give us some STR or an update on the situation?
Thank you!
Flags: needinfo?(gl)
Whiteboard: [qa-triaged]

Some STR could really helps us figure out what exactly should be verified here, if it is still needed.
Could anybody on this bug help us with what have been requested before?
Thanks.

Let's drop the QA verification for this one. Feature landed two years ago and changed a bit since the initial drop.

Flags: qe-verify+
Flags: needinfo?(gl)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: