arni2033 Reporter
	Description • 8 years ago

>>>   My Info:   Win7_64, Nightly 52, 32bit, ID 20161001030430 (2016-10-01)
STR_1:
1. Open https://www.mozilla.org/en-US/firefox/nightly/firstrun/
2. Open devtools -> debugger
3. Press Escape to open split console. Enable filter bar. Close split console
4. Open split console.

AR:
 I see all buttons briefly displayed vertically, one under another. After 0.1s they align horizontally
 If performed on a new profile, STR_1 will leave a blank space on the right side of console.
ER:
 All buttons should be immediately aligned horizontally, shouldn't twitch/blink. No scrollbar or
 free space on the right side of console. Yes, the blank space appears when scrollbar hides   v_v

This is regression from bug 1304178. Regression range:
> https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=37f78aca862224d7151c0fcae1ed8373fe11c83b&tochange=a5510966f80b9b2f5abf59ab32cf4c92d66c60de

	Brian Grinstead [:bgrins] Assignee
	Comment 1 • 7 years ago

Early WIP.  This creates a more simple HTML page and copies over relevant CSS and JS into new files

	Brian Grinstead [:bgrins] Assignee
	Comment 5 • 7 years ago

Honza, I've pushed up a rebased version of the patch.  It still has the double event issue we discussed, likely due to React being loaded twice as you mentioned

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 8 • 7 years ago

Fixed double load of the React module.

Related info:
Honza: https://bugzilla.mozilla.org/show_bug.cgi?id=1341159#c7
Jarda: https://bugzilla.mozilla.org/show_bug.cgi?id=1309866#c80

Final solution Bug 1342297 - Remove ReactDOM event system patch

Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 11 • 7 years ago
mozreview-review

Comment on attachment 8877096 [details]
Bug 1326937 - Patch react-dom to avoid double load

https://reviewboard.mozilla.org/r/148432/#review152950

Hope we can use a non patched react-dom someday

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 12 • 7 years ago

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8d4714714cad14928dea267a98c95dec8892aec&selectedJob=106673800

I am seeing a lot of failures, hope these could help us to fix remaining problems here.

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 13 • 7 years ago

@brian: looks like many tests are failing because of missing `experimentalOutputNode`, what's that?
http://searchfox.org/mozilla-central/rev/d67ef71097da4d1aa344c9d9c672e49a7228e765/devtools/client/webconsole/webconsole.js#569

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 14 • 7 years ago

Test try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=03c9a7a220ed74637427506bef5da1eca77704f8

Patch that sets the `experimentalOutputNode` included. Just to see test failure not related to the output node being used in tests.


Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 15 • 7 years ago

(In reply to Jan Honza Odvarko [:Honza] from comment #13)
> @brian: looks like many tests are failing because of missing
> `experimentalOutputNode`, what's that?
> http://searchfox.org/mozilla-central/rev/
> d67ef71097da4d1aa344c9d9c672e49a7228e765/devtools/client/webconsole/
> webconsole.js#569
> 
> Honza

This is set when the new frontend is loaded in the old document.  With this patch applied:

* All references to experimentalOutputNode in the the new-console-output mochitests should be able to change to just `outputNode` (as set here: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/new-webconsole.js#184)
* All references to experimentalOutputNode can be removed in the old webconsole.js (in fact any conditions with this.NEW_CONSOLE_OUTPUT_ENABLED can be be removed since in this case we are now loading a new document instead).

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 19 • 7 years ago

Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=42e9f4cc28f57dedb72af25e729857fc25c58e02

I am attaching further patch with fixes for test failures. Now memory leaks are fixed too.
A lot less test failures and some of them might be unrelated.

Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 20 • 7 years ago

Since we don't want to deal with the variable view splitter in an html doc, I'm going to block this on Bug 1308566. Although I'm sure there's still more to do here besides waiting on that.

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 25 • 7 years ago

Last try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2968fb9094af8849db5871ecac01f96637700f5&selectedJob=111211970

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 31 • 7 years ago

@Nicolas: I am seeing errors related to reps.css:

INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_parsable_css.js | Got error message for resource://devtools/client/shared/components/reps/reps.css: Unknown property ‘user-select’.  Declaration dropped.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2968fb9094af8849db5871ecac01f96637700f5&selectedJob=111211090

What could be wrong here?

Honza

	Nicolas Chevobbe [:nchevobbe]
	Comment 32 • 7 years ago

I don't really see why it fails now, the reps.css file wasn't changed since a long time (https://hg.mozilla.org/mozilla-central/log/tip/devtools/client/shared/components/reps/reps.css , in February).

It looks more like an error in the test that is failing for me. There's some previous bugs for intermittents in this test.

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 38 • 7 years ago

Last try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8d014d5c1dd6386e3730101966a0f3db6dbe5325
Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 39 • 7 years ago

Last try + patches from bug 1308566 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6f6e7614ca3726a1b69f0b33dd04bdc96b967df7

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 45 • 7 years ago
mozreview-review

Comment on attachment 8877095 [details]
Bug 1326937 - Provide HTML page when running new console frontend;

https://reviewboard.mozilla.org/r/148430/#review161582

	Brian Grinstead [:bgrins] Assignee
	Comment 46 • 7 years ago
mozreview-review

Comment on attachment 8878041 [details]
Bug 1326937 - Fix CSS;

https://reviewboard.mozilla.org/r/149456/#review161622

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_input_focus.js:34
(Diff revision 5)
>    inputNode = hud.jsterm.inputNode;
>    ok(inputNode.getAttribute("focused"), "input node is focused, first");
>  
>    yield waitForBlurredInput(inputNode);
>  
> -  EventUtils.sendMouseEvent({type: "click"}, hud.outputNode);
> +  inputNode.focus();

Should this be instead clicking on the output node?  I believe the intent here is that when you click on the output node it focuses the jsterm.

Maybe `hud.outputNode.click()`?

::: devtools/client/webconsole/webconsole.js:556
(Diff revision 5)
>      this.jsterm = new JSTerm(this);
>      this.jsterm.init();
>  
>      let toolbox = gDevTools.getToolbox(this.owner.target);
>  
>      if (this.NEW_CONSOLE_OUTPUT_ENABLED) {

We should be able to delete this entire condition (and any other references to NEW_CONSOLE_OUTPUT_ENABLED in the file), since we are never running the new console output anymore in this file

	Brian Grinstead [:bgrins] Assignee
	Comment 47 • 7 years ago
mozreview-review

Comment on attachment 8880786 [details]
Bug 1326937 - Fix CSS;

https://reviewboard.mozilla.org/r/152156/#review161882

::: devtools/client/themes/webconsole.css:872
(Diff revision 4)
> +  overflow: auto;
> +  -moz-user-select: text;
> +  position: relative;
> +}
> +
> +:root,

Styles on jsterm, body, :root, and output-container will affect the old frontend as well and I'd prefer to not have to worry about breaking that while it's in the Browser Console.

Can you add a class to the root node in webconsole.xhtml and prefix any specific styles with that class?

	Brian Grinstead [:bgrins] Assignee
	Comment 48 • 7 years ago
mozreview-review

Comment on attachment 8882763 [details]
Bug 1326937 - Avoid an exception due to a missing sidebar;

https://reviewboard.mozilla.org/r/153870/#review161890

::: devtools/client/webconsole/jsterm.js:588
(Diff revision 3)
>     *        the variables view window after open, |false| otherwise.
>     * @return object
>     *         A promise object that is resolved when the variables view has
>     *         opened. The new variables view instance is given to the callbacks.
>     */
>    openVariablesView: function (options) {

Nit: does it work to bail out at the top of this function with something like: 

```
if (!this.hud.document.querySelector("#webconsole-sidebar") {
  return Promise.resolve(null);
}
```

instead of propogating the conditions into `onContainerReady`, `_createSidebar`, and `_addVariablesViewSidebarTab`?

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 55 • 7 years ago

(In reply to Brian Grinstead [:bgrins] from comment #46)
> Comment on attachment 8878041 [details]
> Bug 1326937 - Fix test failures;
> > -  EventUtils.sendMouseEvent({type: "click"}, hud.outputNode);
> > +  inputNode.focus();
> 
> Should this be instead clicking on the output node?  I believe the intent
> here is that when you click on the output node it focuses the jsterm.
I found real bug and the test now passes without any modifications.

> >      if (this.NEW_CONSOLE_OUTPUT_ENABLED) {
> 
> We should be able to delete this entire condition (and any other references
> to NEW_CONSOLE_OUTPUT_ENABLED in the file), since we are never running the
> new console output anymore in this file
Done 

(In reply to Brian Grinstead [:bgrins] from comment #47)
> Comment on attachment 8880786 [details]
> Bug 1326937 - Fix CSS;
> ::: devtools/client/themes/webconsole.css:872
> (Diff revision 4)
> > +  overflow: auto;
> > +  -moz-user-select: text;
> > +  position: relative;
> > +}
> > +
> > +:root,
> 
> Styles on jsterm, body, :root, and output-container will affect the old
> frontend as well and I'd prefer to not have to worry about breaking that
> while it's in the Browser Console.
> 
> Can you add a class to the root node in webconsole.xhtml and prefix any
> specific styles with that class?
I used body for this purpose since it's only available in .xhtml, no?
Anyway, I replaced :root by html.


(In reply to Brian Grinstead [:bgrins] from comment #48)
> Comment on attachment 8882763 [details]
> Bug 1326937 - Avoid an exception due to a missing sidebar;
> Nit: does it work to bail out at the top of this function with something
> like: 
> 
> ```
> if (!this.hud.document.querySelector("#webconsole-sidebar") {
>   return Promise.resolve(null);
> }
> ```
> 
> instead of propogating the conditions into `onContainerReady`,
> `_createSidebar`, and `_addVariablesViewSidebarTab`?
Good point, done.

Thanks!
Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 56 • 7 years ago
mozreview-review

Comment on attachment 8878041 [details]
Bug 1326937 - Fix CSS;

https://reviewboard.mozilla.org/r/149456/#review162102

Nice. Can you fold this into the first patch before we land?

	Brian Grinstead [:bgrins] Assignee
	Comment 57 • 7 years ago
mozreview-review

Comment on attachment 8877095 [details]
Bug 1326937 - Provide HTML page when running new console frontend;

https://reviewboard.mozilla.org/r/148430/#review162106

::: testing/talos/talos/tests/devtools/addon/content/damp.js:27
(Diff revision 8)
>    this._heapSnapshotFilePath = null;
>    // HeapSnapshot instance. Set by readHeapSnapshot, used by takeCensus.
>    this._snapshot = null;
>  
>    // Use the old console for now: https://bugzilla.mozilla.org/show_bug.cgi?id=1306780
> -  Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", false);
> +  // Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", false);

Please revert the change to this file

	Brian Grinstead [:bgrins] Assignee
	Comment 58 • 7 years ago
mozreview-review

Comment on attachment 8882763 [details]
Bug 1326937 - Avoid an exception due to a missing sidebar;

https://reviewboard.mozilla.org/r/153870/#review162108

Can you fold this into the first patch (along with the test fixes)?

	Brian Grinstead [:bgrins] Assignee
	Comment 59 • 7 years ago
mozreview-review

Comment on attachment 8880786 [details]
Bug 1326937 - Fix CSS;

https://reviewboard.mozilla.org/r/152156/#review162104

::: devtools/client/themes/webconsole.css:406
(Diff revision 5)
>    background-position: -60px -36px;
>  }
>  
>  /* JSTerm Styles */
> +
> +#jsterm-wrapper,

Please prefix all of these with `html` so that it doesn't affect the old frontend

::: devtools/client/themes/webconsole.css
(Diff revision 5)
>  
> -/*
> - * Open DOMNode in inspector button. Style need to be reset in the new
> - * console since its style is already defined in reps.css .
> - */
> -.webconsole-output-wrapper .open-inspector {

Why is this workaround not needed anymore?

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 63 • 7 years ago

(In reply to Brian Grinstead [:bgrins] from comment #56)
> Comment on attachment 8878041 [details]
> Bug 1326937 - Fix CSS;
> Nice. Can you fold this into the first patch before we land?
Done

(In reply to Brian Grinstead [:bgrins] from comment #57)
> Comment on attachment 8877095 [details]
> Bug 1326937 - Provide HTML page when running new console frontend;
> > -  Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", false);
> > +  // Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", false);
> 
> Please revert the change to this file
Done

Shouldn't we enable damp tests for the new front end?

(In reply to Brian Grinstead [:bgrins] from comment #58)
> Comment on attachment 8882763 [details]
> Bug 1326937 - Avoid an exception due to a missing sidebar;
> Can you fold this into the first patch (along with the test fixes)?
Done

(In reply to Brian Grinstead [:bgrins] from comment #59)
> Comment on attachment 8880786 [details]
> Bug 1326937 - Fix CSS;
> > +
> > +#jsterm-wrapper,
> 
> Please prefix all of these with `html` so that it doesn't affect the old
> frontend
Done

> > -/*
> > - * Open DOMNode in inspector button. Style need to be reset in the new
> > - * console since its style is already defined in reps.css .
> > - */
> > -.webconsole-output-wrapper .open-inspector {
> 
> Why is this workaround not needed anymore?
This caused the inspector icon to be huge and covering most of the console content. But I am not seeing the problem anymore, so I reverted the change.

Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 64 • 7 years ago

(In reply to Jan Honza Odvarko [:Honza] from comment #63)
> (In reply to Brian Grinstead [:bgrins] from comment #57)
> > Comment on attachment 8877095 [details]
> > Bug 1326937 - Provide HTML page when running new console frontend;
> > > -  Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", false);
> > > +  // Services.prefs.setBoolPref("devtools.webconsole.new-frontend-enabled", false);
> > 
> > Please revert the change to this file
> Done
> 
> Shouldn't we enable damp tests for the new front end?

Yes, but let's wait until we flip the pref for non-nightly builds (or at least, let's do it in a different bug than this)

	Brian Grinstead [:bgrins] Assignee
	Comment 65 • 7 years ago
mozreview-review

Comment on attachment 8877095 [details]
Bug 1326937 - Provide HTML page when running new console frontend;

https://reviewboard.mozilla.org/r/148430/#review162620

Thank you, this looks great!

	Brian Grinstead [:bgrins] Assignee
	Comment 66 • 7 years ago

This should be ready to land, with a good try push

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 70 • 7 years ago

One failing test fixed:
devtools/client/webconsole/new-console-output/test/mochitest/browser_webconsole_console_dir.js

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97c2fe9f5d25357510cb7864f8960502444c47e2

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 71 • 7 years ago

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=05370418b06ff62354cb0d84641f3987779ad187

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 72 • 7 years ago

Try (all OSes)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0223dc25af4bf65becb8dbaa69498f5b34729eb4

Honza

	Pulsebot
	Comment 73 • 7 years ago

Pushed by jodvarko@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/227ff88b9848
Provide HTML page when running new console frontend; r=bgrins
https://hg.mozilla.org/integration/autoland/rev/57701c371c8d
Patch react-dom to avoid double load r=bgrins
https://hg.mozilla.org/integration/autoland/rev/d99559a7adc7
Fix CSS; r=bgrins

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 74 • 7 years ago

Backed out for timing out in browser_ext_devtools_inspectedWindow_eval_bindings.js:

https://hg.mozilla.org/integration/autoland/rev/99c1d1e02a53d0f1e44fdca3057c50c3a6e3fc18
https://hg.mozilla.org/integration/autoland/rev/8418d3ebb3a0d47686c61e62a68c0fb0da7b5c25
https://hg.mozilla.org/integration/autoland/rev/6a3d7b1a87e0243e2240fee60ca1f31795995d39

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d99559a7adc7cc556575905fc6ddfe9b650ea6bb&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=114927927&repo=autoland

[task 2017-07-17T18:02:43.340796Z] 18:02:43     INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_eval_bindings.js | Got the expected eval result - {"evalResult":"BODY","errorResult":"undefined"} deepEqual {"evalResult":"BODY","errorResult":"undefined"} - 
[task 2017-07-17T18:02:43.345182Z] 18:02:43     INFO - Toolbox switched back to the styleeditor panel
[task 2017-07-17T18:02:43.349233Z] 18:02:43     INFO - Test inspectedWindow.eval inspect() binding called for a DOM element
[task 2017-07-17T18:02:43.351046Z] 18:02:43     INFO - Buffered messages logged at 18:02:01
[task 2017-07-17T18:02:43.356386Z] 18:02:43     INFO - Wait for the toolbox to switch to the inspector and the expected node has been selected
[task 2017-07-17T18:02:43.362235Z] 18:02:43     INFO - TEST-PASS | browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_eval_bindings.js | The expected DOM node has been selected in the inspector - 
[task 2017-07-17T18:02:43.372608Z] 18:02:43     INFO - Toolbox has been switched to the inspector as expected
[task 2017-07-17T18:02:43.376195Z] 18:02:43     INFO - Test inspectedWindow.eval inspect() binding called for a JS object
[task 2017-07-17T18:02:43.378137Z] 18:02:43     INFO - Wait for the split console to be opened and the JS object inspected
[task 2017-07-17T18:02:43.383217Z] 18:02:43     INFO - Console message: [JavaScript Warning: "Unknown property ‘user-select’.  Declaration dropped." {file: "resource://devtools/client/shared/components/reps/reps.css" line: 212 column: 13 source: "  user-select: none;"}]
[task 2017-07-17T18:02:43.385301Z] 18:02:43     INFO - Buffered messages finished
[task 2017-07-17T18:02:43.391350Z] 18:02:43     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_eval_bindings.js | Test timed out - 
[task 2017-07-17T18:02:43.392912Z] 18:02:43     INFO - Not taking screenshot here: see the one that was previously logged
[task 2017-07-17T18:02:43.401262Z] 18:02:43     INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_eval_bindings.js | Extension left running at test shutdown - 
[task 2017-07-17T18:02:43.402852Z] 18:02:43     INFO - Stack trace:
[task 2017-07-17T18:02:43.404390Z] 18:02:43     INFO -     chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:109
[task 2017-07-17T18:02:43.405936Z] 18:02:43     INFO -     chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:390
[task 2017-07-17T18:02:43.407515Z] 18:02:43     INFO -     timeoutFn@chrome://mochikit/content/browser-test.js:858:9
[task 2017-07-17T18:02:43.409738Z] 18:02:43     INFO -     setTimeout handler*Tester_execTest@chrome://mochikit/content/browser-test.js:820:9
[task 2017-07-17T18:02:43.411277Z] 18:02:43     INFO -     Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:670:7
[task 2017-07-17T18:02:43.413370Z] 18:02:43     INFO -     SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:795:59

	Brian Grinstead [:bgrins] Assignee
	Comment 75 • 7 years ago

Since we need to re-land anyway: I noticed that the first patch has me as the author (I think since we folded your work into the original WIP). Honza, can you update that to be you?

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 80 • 7 years ago

(In reply to Brian Grinstead [:bgrins] from comment #75)
> Since we need to re-land anyway: I noticed that the first patch has me as
> the author (I think since we folded your work into the original WIP). Honza,
> can you update that to be you?
Sure, done.

Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 81 • 7 years ago
mozreview-review

Comment on attachment 8887458 [details]
Bug 1326937 - Fix persistent logs;

https://reviewboard.mozilla.org/r/158294/#review163546

::: browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_eval_bindings.js
(Diff revision 1)
>      is(objectType, "object", "The inspected object has the expected type");
>      Assert.deepEqual(Object.keys(objectPreviewProperties), ["testkey"],
>                       "The inspected object has the expected preview properties");
>    })();
>  
> -  const inspectJSObjectPromise = extension.awaitMessage(`inspectedWindow-eval-result`);

I don't think removing this code is the right solution.  I tested locally and I'm not seeing the object in the console (I do see the split console get opened though).

The problem here I think is that 'inspect()' isn't working in the xhtml doc (due to lack of vview). We need to at least wait for Bug 1379570 (currently on autoland), and confirm this test works with that patch applied.

	Brian Grinstead [:bgrins] Assignee
	Comment 92 • 7 years ago
mozreview-review

Comment on attachment 8887458 [details]
Bug 1326937 - Fix persistent logs;

https://reviewboard.mozilla.org/r/158294/#review164652

::: browser/components/extensions/test/browser/browser_ext_devtools_inspectedWindow_eval_bindings.js:140
(Diff revision 3)
>      });
>  
> -    const objectType = options.objectActor.type;
> -    const objectPreviewProperties = options.objectActor.preview.ownProperties;
> -    is(objectType, "object", "The inspected object has the expected type");
> -    Assert.deepEqual(Object.keys(objectPreviewProperties), ["testkey"],
> +    let objectInspectors = [...messageNode.querySelectorAll(".tree")];
> +    is(objectInspectors.length, 1, "There is the expected number of object inspectors");
> +
> +    const [objectInspector] = objectInspectors;

IMO all of the changes after this can be removed - we have devtools tests making sure inspect/OI work so here we can be satisfied once we know the OI exists

::: devtools/client/webconsole/jsterm.js:413
(Diff revision 3)
>      if (WebConsoleUtils.isActorGrip(result)) {
>        msg._objectActors.add(result.actor);
>      }
>    },
>  
>    inspectObjectActor: function (objectActor) {

Can you also revert the changes to executeResultCallback from https://hg.mozilla.org/mozilla-central/rev/4dd3191931e0#l1.2 so that we call inspectObjectActor there as well?

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 97 • 7 years ago

(In reply to Brian Grinstead [:bgrins] from comment #92)
> Comment on attachment 8887458 [details]
> Bug 1326937 - Fix persitent logs;
> 
> https://reviewboard.mozilla.org/r/158294/#review164652
This patch will be handled separately in bug Bug 1382690 

Honza

	Brian Grinstead [:bgrins] Assignee
	Comment 98 • 7 years ago
mozreview-review

Comment on attachment 8887458 [details]
Bug 1326937 - Fix persistent logs;

https://reviewboard.mozilla.org/r/158294/#review164662

	Brian Grinstead [:bgrins] Assignee
	Comment 100 • 7 years ago

Try push with the fix from bug 1382690 applied: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ee7680a90c22eb9d18597f9607ee8e3daa1edb1e

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 105 • 7 years ago

Patch updated to fix failure on try.

New try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=10922b8f9ee10dd98541e1fa2a0b268e61a90831

Honza

	Jan Honza Odvarko [:Honza] (always need-info? me)
	Comment 106 • 7 years ago

Ah, yet forgot patch from bug 1382690

New try:
 https://treeherder.mozilla.org/#/jobs?repo=try&revision=916f6549d37210a23eaa68c689e90e55c87ad350

Honza

	Pulsebot
	Comment 107 • 7 years ago

Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f2e14280fab
Provide HTML page when running new console frontend; r=bgrins
https://hg.mozilla.org/integration/autoland/rev/5612970bf698
Patch react-dom to avoid double load r=bgrins
https://hg.mozilla.org/integration/autoland/rev/499fa676cb6d
Fix CSS; r=bgrins
https://hg.mozilla.org/integration/autoland/rev/d941b592133f
Fix persistent logs; r=bgrins

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 108 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/2f2e14280fab
https://hg.mozilla.org/mozilla-central/rev/5612970bf698
https://hg.mozilla.org/mozilla-central/rev/499fa676cb6d
https://hg.mozilla.org/mozilla-central/rev/d941b592133f

	Iulia Cristescu, QA [:JuliaC] (away, please needinfo? cornel.ionce@softvision.ro)
	Comment 109 • 7 years ago

Reproduced the initial issue on 53.0a1 (2017-01-01). I can confirm the bug is verified fixed on latest Nightly 56.0a1 (2017-07-24), using Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.11.6.