A small speed up can be made to the VariablesView so that it only computes _symbolicName on-demand via a getter. See bug 947710 comment 10 for more info.

Hi Ryan, Can I work on this bug? what should I do? .. Is there any thing to learn as I'm new to this .. :)

Hi, Feel free to follow this guide to get into contributing to Firefox: https://developer.mozilla.org/en-US/docs/Introduction

Sure, thanks for your interest! There is also a more Dev Tools focused guide[1] to getting started with the code base. This bug involves changes to VariablesView.jsm[2]. In that file, you'll see several places where _symbolicName is computed at initialization time. We'd like to change that so it can happen on-demand, similar to symbolicPath[3], which was added recently (DXR and MXR aren't up to date with this change yet since it's so recent). Please let me know if you have any questions! [1]: https://wiki.mozilla.org/DevTools/Hacking [2]: https://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/widgets/VariablesView.jsm [3]: http://hg.mozilla.org/integration/fx-team/file/c85cbcb0e170/browser/devtools/shared/widgets/VariablesView.jsm#l2304

So you mean, changed >get symbolicName() this._symbolicName, in to > get symbolicName() { > if (this._symbolicName) { return this._symbolicName; > } > },

(In reply to Dulanja Wijethunga [:dwij] from comment #4) > So you mean, > changed > >get symbolicName() this._symbolicName, > > in to > > > get symbolicName() { > > if (this._symbolicName) { > return this._symbolicName; > > } > > }, Well, that's part of it. We want to only compute _symbolicName the first time is accessed. So you'd need to look at the places where it is built up now during initialization, and convert them to a technique that happens later, on first access. This is similar to how _buildSymbolicPath works to build up the path recursively. It's only called once on first access via the getter for symbolicPath.

Sorry for the late response! (last week I had a exam) (In reply to J. Ryan Stinnett [:jryans] from comment #5) > Well, that's part of it. We want to only compute _symbolicName the first > time is accessed. I suppose you mean that I have to add _buildSymbolicName function like below >So you'd need to look at the places where it is built up > now during initialization, I found two [1]https://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/widgets/VariablesView.jsm#2132 [2]https://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/widgets/VariablesView.jsm#2867 are there more? > and convert them to a technique that happens > later, on first access. This is similar to how _buildSymbolicPath works to > build up the path recursively. It's only called once on first access via > the getter for symbolicPath. can you explain what happens by the symbolicName I really couldn't understand what _buildSymbolicPath does. If I have to add _buildSymbolicName what would be its parameter string or a array? what it returns ?

(In reply to Dulanja Wijethunga [:dwij] from comment #6) > Sorry for the late response! (last week I had a exam) No worries, there is no rush! :) Sorry for my delayed response as well, the Mozilla US was closed on Monday. > (In reply to J. Ryan Stinnett [:jryans] from comment #5) > > Well, that's part of it. We want to only compute _symbolicName the first > > time is accessed. > > I suppose you mean that I have to add _buildSymbolicName function like below Yes, I would expect you would need to add a new _buildSymbolicName function that is called from a getter for symbolicName, but only the first time. See the getter for symbolicPath for ideas. > >So you'd need to look at the places where it is built up > > now during initialization, > > I found two > [1]https://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/ > widgets/VariablesView.jsm#2132 > [2]https://mxr.mozilla.org/mozilla-central/source/browser/devtools/shared/ > widgets/VariablesView.jsm#2867 > > are there more? No, I believe those are the only ones. > > and convert them to a technique that happens > > later, on first access. This is similar to how _buildSymbolicPath works to > > build up the path recursively. It's only called once on first access via > > the getter for symbolicPath. > > can you explain what happens by the symbolicName I really couldn't > understand what _buildSymbolicPath does. Variable and Property instances in the VariablesView and in a tree structure, so _buildSymbolicPath starts at the current Variable / Property node, and uses recursion to visit each parent, eventually building up the sequence of names visited in an array. > If I have to add _buildSymbolicName what would be its parameter string or a > array? what it returns ? This is new function would likely use recursion and look quite similar to _buildSymbolicPath, but in this case you are building up a single string. Try some test cases to see to what symbolicName typically contains. Since you are not trying to change the value, just how it is computed, you'll want to ensure you are producing the same result as before.

Dulanja - This looks pretty close, are you still working on it?

(In reply to Mike Hoye [:mhoye] from comment #8) > Dulanja - This looks pretty close, are you still working on it? Hi Mike, well these days I'm busy with my academics.. so you can continue if you want :)

it's been six months since any real activity. Going to throw this back into the unassigned category. Dulanja if you think you can get back to it, please comment in the bug or reassign yourself. Thanks!

There is a patch with possible fix. Need a feedback

Thanks for the patch! To request feedback, you set the feedback flag "?" and fill in the email of who you are requesting it from. That person then sets the status to -, +, or clears it after responding. I'll fix this up for the current patch.

Comment on attachment 8455505 [details] [diff] [review] symbolicNamePerf.patch ># HG changeset patch ># Parent 340b19c14d3d388b7ae1a15bff9695cb0141ac0a ># User railscard <railscard@gmail.com> >diff --git a/browser/devtools/shared/widgets/VariablesView.jsm b/browser/devtools/shared/widgets/VariablesView.jsm >--- a/browser/devtools/shared/widgets/VariablesView.jsm >+++ b/browser/devtools/shared/widgets/VariablesView.jsm >@@ -18,17 +18,17 @@ const PAGE_SIZE_MAX_JUMPS = 30; > const SEARCH_ACTION_MAX_DELAY = 300; // ms > const ITEM_FLASH_DURATION = 300 // ms > > Cu.import("resource://gre/modules/Services.jsm"); > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > Cu.import("resource:///modules/devtools/ViewHelpers.jsm"); > Cu.import("resource://gre/modules/devtools/event-emitter.js"); > Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm"); > > let {Promise: promise} = Cu.import("resource://gre/modules/Promise.jsm", {}); > > XPCOMUtils.defineLazyModuleGetter(this, "devtools", > "resource://gre/modules/devtools/Loader.jsm"); > > XPCOMUtils.defineLazyModuleGetter(this, "PluralForm", > "resource://gre/modules/PluralForm.jsm"); > >@@ -2314,22 +2314,40 @@ Variable.prototype = Heritage.extend(Sco > let descriptor = Object.create(aDescriptor); > descriptor.get = VariablesView.getGrip(aDescriptor.get); > descriptor.set = VariablesView.getGrip(aDescriptor.set); > > return this.addItem(aName, descriptor); > }, > > /** >- * Gets this variable's path to the topmost scope in the form of a string >+ * Gets this variable's path to the topmost scope >+ * @see Variable._buildSymbolicName >+ * @return string >+ */ >+ get symbolicName() { >+ if (this._symbolicName) { >+ return this._symbolicName; >+ } >+ this._symbolicName = this._buildSymbolicName(); >+ return this._symbolicName; >+ }, >+ >+ /** >+ * Build this variable's name to the topmost scope in the form of a string > * meant for use via eval() or a similar approach. > * For example, a symbolic name may look like "arguments['0']['foo']['bar']". > * @return string > */ >- get symbolicName() this._symbolicName, >+ _buildSymbolicName: function(namePath = "") { >+ if (this.name) { >+ namePath = this.name + namePath; >+ } >+ return namePath; >+ }, > > /** > * Gets this variable's symbolic path to the topmost scope. > * @return array > * @see Variable._buildSymbolicPath > */ > get symbolicPath() { > if (this._symbolicPath) { >@@ -2969,17 +2987,17 @@ Variable.prototype = Heritage.extend(Sco > if (!this._variablesView.preventDisableOnChange) { > this._disable(); > } > this.ownerView.new(this, aKey, aValue); > } > }, e); > }, > >- _symbolicName: "", >+ _symbolicName: null, > _symbolicPath: null, > _absoluteName: "", > _initialDescriptor: null, > _separatorLabel: null, > _valueLabel: null, > _spacer: null, > _editNode: null, > _deleteNode: null, >@@ -3000,25 +3018,39 @@ Variable.prototype = Heritage.extend(Sco > * The variable to contain this property. > * @param string aName > * The property's name. > * @param object aDescriptor > * The property's descriptor. > */ > function Property(aVar, aName, aDescriptor) { > Variable.call(this, aVar, aName, aDescriptor); >- this._symbolicName = aVar._symbolicName + "[\"" + aName + "\"]"; >+ this._symbolicName = null; > this._absoluteName = aVar._absoluteName + "[\"" + aName + "\"]"; > } > > Property.prototype = Heritage.extend(Variable.prototype, { > /** > * The class name applied to this property's target element. > */ >- targetClassName: "variables-view-property variable-or-property" >+ targetClassName: "variables-view-property variable-or-property", >+ >+ /** >+ * @see Variable._buildSymbolicName >+ * @return string >+ */ >+ _buildSymbolicName: function(namePath = "") { >+ if (this.name) { >+ namePath = "[\"" + this.name + "\"]" + namePath; >+ if (this.ownerView instanceof Variable) { >+ return this.ownerView._buildSymbolicName(namePath); >+ } >+ } >+ return namePath; >+ } > }); > > /** > * A generator-iterator over the VariablesView, Scopes, Variables and Properties. > */ > VariablesView.prototype["@@iterator"] = > Scope.prototype["@@iterator"] = > Variable.prototype["@@iterator"] = >@@ -4047,9 +4079,9 @@ EditableNameAndValue.prototype = Heritag > valueEditable.deactivate(); > }; > }, > > _save: function(e) { > // Both _save and _next activate the value edit box. > this._next(e); > } >-}); >+}); >\ No newline at end of file

railscard, I had already fixed the flags... Please reset it to "?" for feedback, and enter my email (jryans@gmail.com).

(In reply to J. Ryan Stinnett [:jryans] from comment #12) > Thanks for the patch! To request feedback, you set the feedback flag "?" > and fill in the email of who you are requesting it from. That person then > sets the status to -, +, or clears it after responding. > > I'll fix this up for the current patch. Thanks for this! Guess I'm doing something wrong :) I'm new to bugzilla, how can I remove my last comment (#c13) from this thread?

(In reply to railscard from comment #15) > (In reply to J. Ryan Stinnett [:jryans] from comment #12) > > Thanks for the patch! To request feedback, you set the feedback flag "?" > > and fill in the email of who you are requesting it from. That person then > > sets the status to -, +, or clears it after responding. > > > > I'll fix this up for the current patch. > > Thanks for this! Guess I'm doing something wrong :) I'm new to bugzilla, how > can I remove my last comment (#c13) from this thread? There is no delete for comments, you can only hide bad comments by "tagging" them with certain special words, like "typo". See https://wiki.mozilla.org/BMO/comment_tagging. Also, you need to re-correct the feedback flag still as you un-did my first attempt to fix it for you.

(In reply to J. Ryan Stinnett [:jryans] from comment #16) > There is no delete for comments, you can only hide bad comments by "tagging" > them with certain special words, like "typo". See > https://wiki.mozilla.org/BMO/comment_tagging. > > Also, you need to re-correct the feedback flag still as you un-did my first > attempt to fix it for you. Fixed.

Comment on attachment 8455505 [details] [diff] [review] symbolicNamePerf.patch Review of attachment 8455505 [details] [diff] [review]: ----------------------------------------------------------------- I've suggested a slightly different approach in the comments, so take a look at that. There are several other places in VariablesView that read from |_symbolicName| currently, so be sure to change those to |symbolicName| so they trigger your new getter, instead of reading null everywhere. ::: browser/devtools/shared/widgets/VariablesView.jsm @@ +22,5 @@ > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > Cu.import("resource:///modules/devtools/ViewHelpers.jsm"); > Cu.import("resource://gre/modules/devtools/event-emitter.js"); > Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm"); > + Undo this change, since it's not related to this bug. @@ +2326,5 @@ > + get symbolicName() { > + if (this._symbolicName) { > + return this._symbolicName; > + } > + this._symbolicName = this._buildSymbolicName(); If you read earlier comments on this bug, I did indeed suggest adding |_buildSymbolicName|, but I think there's a even simpler approach that would be better here. Rather than having a second recursive function (|_buildSymbolicName| in addition to |_buildSymbolicPath|), you can use the work that is done for |symbolicPath| directly, since it gives you a real array. To compute |_symbolicName|, you'd get |this.symbolicPath| and then loop over the array, and appending a stringified version of each value of the array. I think this is better overall. Sorry for making this more confusing from my earlier comments. @@ +2328,5 @@ > + return this._symbolicName; > + } > + this._symbolicName = this._buildSymbolicName(); > + return this._symbolicName; > + }, Watch out for trailing spaces like this one. @@ +2336,5 @@ > * meant for use via eval() or a similar approach. > * For example, a symbolic name may look like "arguments['0']['foo']['bar']". > * @return string > */ > + _buildSymbolicName: function(namePath = "") { This won't be needed with the approach I suggested above. @@ +3022,5 @@ > * The property's descriptor. > */ > function Property(aVar, aName, aDescriptor) { > Variable.call(this, aVar, aName, aDescriptor); > + this._symbolicName = null; Property extends Variable, so this would already be null after your change on line 2977. You should be able to just remove this line. However, you also need to change the Variable constructor to remove the line setting |_symbolicName| there too. @@ +3036,5 @@ > + /** > + * @see Variable._buildSymbolicName > + * @return string > + */ > + _buildSymbolicName: function(namePath = "") { This won't be needed with the approach I suggested above. @@ +3038,5 @@ > + * @return string > + */ > + _buildSymbolicName: function(namePath = "") { > + if (this.name) { > + namePath = "[\"" + this.name + "\"]" + namePath; You can use this same stringifying approach when looping over the |symbolicPath|.

Also, the web console is the main user of |symbolicName| today, so be sure to run those tests on your machine after making your changes.

(In reply to J. Ryan Stinnett [:jryans] from comment #18) > Comment on attachment 8455505 [details] [diff] [review] > symbolicNamePerf.patch > > There are several other places in VariablesView that read from |_symbolicName| currently, so be sure > to change those to |symbolicName| so they trigger your new getter, instead of reading null everywhere. Sorry for my inattention! > > I've suggested a slightly different approach in the comments, so take a look > at that. > > Rather than having a second recursive function (|_buildSymbolicName| in > addition to |_buildSymbolicPath|), you can use the work that is done for > |symbolicPath| directly, since it gives you a real array. To compute > |_symbolicName|, you'd get |this.symbolicPath| and then loop over the array, > and appending a stringified version of each value of the array. > > I think this is better overall. Sorry for making this more confusing from > my earlier comments. > In case we have something like x = {y: {z: ""}}, symbolic path for property "z" should be x["y"]["z"], right? I'm not sure how to achieve this by simply stringifying an array from the symbolicPath. Maybe I misunderstood something?

(In reply to railscard from comment #20) > (In reply to J. Ryan Stinnett [:jryans] from comment #18) > > Comment on attachment 8455505 [details] [diff] [review] > > symbolicNamePerf.patch > > > > There are several other places in VariablesView that read from |_symbolicName| currently, so be sure > to change those to |symbolicName| so they trigger your new getter, instead of reading null everywhere. > > Sorry for my inattention! > > > > > I've suggested a slightly different approach in the comments, so take a look > > at that. > > > > Rather than having a second recursive function (|_buildSymbolicName| in > > addition to |_buildSymbolicPath|), you can use the work that is done for > > |symbolicPath| directly, since it gives you a real array. To compute > > |_symbolicName|, you'd get |this.symbolicPath| and then loop over the array, > > and appending a stringified version of each value of the array. > > > > I think this is better overall. Sorry for making this more confusing from > > my earlier comments. > > > In case we have something like x = {y: {z: ""}}, symbolic path for property > "z" should be x["y"]["z"], right? I'm not sure how to achieve this by simply > stringifying an array from the symbolicPath. > > Maybe I misunderstood something? I think the main reason this is confusing is the comment above |get symbolicName()| suggests that the variable name will appear before the ["y"]["z"] part, but the only caller of |symbolicName| today is the Console (webconsole.js), and it doesn't expect to handle x["y"]["z"], only ["y"]["z"]. So, we are changing things slightly, but it still matches want the caller of |symbolicName| expects.

Comment on attachment 8455987 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8455987 [details] [diff] [review]: ----------------------------------------------------------------- Please add a comment message that summarizes the change you've made (don't simple repeat the bug title) using the format "Bug 123456 - Change this thing to work better by doing something. r=jryans". See the guide[1] for more details. Only a few comments this time. Thanks for the quick update! :D For the next patch, use the review flag. Assuming it looks good (meaning I give an r+), then I'll push it to our test environment to run various test suites against it. [1]: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F ::: browser/devtools/shared/widgets/VariablesView.jsm @@ +2320,5 @@ > > /** > * Gets this variable's path to the topmost scope in the form of a string > * meant for use via eval() or a similar approach. > * For example, a symbolic name may look like "arguments['0']['foo']['bar']". Update this comment to remove "arguments" since that wouldn't be included. @@ +3016,5 @@ > * The property's descriptor. > */ > function Property(aVar, aName, aDescriptor) { > Variable.call(this, aVar, aName, aDescriptor); > + this._symbolicName = null; I think you can remove this line entirely.

Bug 956760 - Improve VariablesView performace by disabling _symbolicName calculation at initialization time. r=jryans

Comment on attachment 8456481 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8456481 [details] [diff] [review]: ----------------------------------------------------------------- Your code looks great to me! But the commit message needs to be part of the patch attachment itself, not as a bug comment. See the guide[1] for more info, or ping me on IRC. [1]: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

(In reply to J. Ryan Stinnett [:jryans] from comment #25) > Comment on attachment 8456481 [details] [diff] [review] > symbolicNamePerfv2.patch > > Review of attachment 8456481 [details] [diff] [review]: > ----------------------------------------------------------------- > > Your code looks great to me! But the commit message needs to be part of the > patch attachment itself, not as a bug comment. See the guide[1] for more > info, or ping me on IRC. > > [1]: > https://developer.mozilla.org/en-US/docs/ > Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check- > in_for_me.3F Great, thank you for feedback! I've added commit message to my attachment

Comment on attachment 8456848 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8456848 [details] [diff] [review]: ----------------------------------------------------------------- Great, looks good! I've submitted this to our try server to run test suites. If everything looks good, we can check it in after that. Try: https://tbpl.mozilla.org/?tree=Try&rev=f6d8bb8cac88

Looks like there were a few test failures, and surprise, I was wrong, we do indeed need to preserve the first name string without brackets. I'll take a look at the initial approach you submitted again. Sorry for the trouble!

Okay, I think the algorithm of your first patch looks correct. So, if you clean up the patch (ensure all users of _symbolicName trigger the getter, etc.) and ask for review, we can try that approach again. Also, you should try running the webconsole and debugger tests on your own machine to ensure they pass. It's a good idea to run tests locally before submitting a patch for any bug. You can do this with |./mach mochitest-devtools browser/devtools/debugger| as an example. See the Hacking[1] page for more info. Again, sorry for my own confusion! [1]: https://wiki.mozilla.org/DevTools/Hacking#Running_the_Developer_Tools_Tests

This solution looks a little bit clean

Comment on attachment 8457097 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8457097 [details] [diff] [review]: ----------------------------------------------------------------- Cool, this seems correct to me! Try: https://tbpl.mozilla.org/?tree=Try&rev=c3bb9ea5d27b

Can we do this for _absoluteName as well?

(In reply to Victor Porof [:vporof][:vp] from comment #33) > Can we do this for _absoluteName as well? Seems reasonable. railscard, you could add this to your patch here, or file a follow up bug and possibly work on it there if you'd like to.

The tests look good for your patch here, so you can request that it be checked in by adding the keyword "checkin-needed" in the Keywords field at the top of the bug. Or, if you'd like to address Victor's request in this bug, I'm happy to look at a new patch here.

Comment on attachment 8458284 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8458284 [details] [diff] [review]: ----------------------------------------------------------------- Code looks good to me. Just a few style nits to address. Once you've addressed these, I'll push your new patch to Try for a test run. ::: browser/devtools/shared/widgets/VariablesView.jsm @@ +1710,5 @@ > /** > + * Gets the full name of this item. > + * return @string > + */ > + get absoluteName() this._nameString, It's true that this syntax is used in other places in this file, but it's because this is an older file. This is a Mozilla-specific syntax, and these days we want to use standard getter syntax which looks like: get absoluteName() { return this._nameString; } Existing cases should be left alone to not confuse this patch, but for new ones or something that you need to update as part of this work, it's good to convert to the new syntax. @@ +2327,5 @@ > * meant for use via eval() or a similar approach. > * For example, a symbolic name may look like "arguments['0']['foo']['bar']". > * @return string > */ > + get symbolicName() this.name, Update the getter syntax here as well. @@ +2339,5 @@ > + return this._absoluteName > + } > + > + this._absoluteName = this.ownerView.absoluteName + "[\"" + this.name + "\"]"; > + return this._absoluteName; Nit: Remove trailing space

(In reply to J. Ryan Stinnett [:jryans] from comment #37) > Just a few style nits to address. Thanks for the feedback! Fixed.

Comment on attachment 8458870 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8458870 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! Let's see how the tests go. Try: https://tbpl.mozilla.org/?tree=Try&rev=2a8e785d6477

Okay, tests look good. Assuming you're ready for this to be checked in, you should set "checkin-needed" in Keywords field at the top.

Comment on attachment 8458870 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8458870 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/shared/widgets/VariablesView.jsm @@ +1710,5 @@ > /** > + * Gets the full name of this item. > + * return @string > + */ > + get absoluteName() { Why does a scope get an absolute name? It did not have one before. Only variables and properties do. Please remove this. @@ +2330,5 @@ > * For example, a symbolic name may look like "arguments['0']['foo']['bar']". > * @return string > */ > + get symbolicName() { > + return this.name Nit: semicolon. Please return this._nameString to avoid calling an additional getter. @@ +2339,5 @@ > + * @return string > + */ > + get absoluteName() { > + if (this._absoluteName) { > + return this._absoluteName Nit: semicolon. @@ +2342,5 @@ > + if (this._absoluteName) { > + return this._absoluteName > + } > + > + this._absoluteName = this.ownerView.absoluteName + "[\"" + this.name + "\"]"; This should be this._absoluteName = this.ownerView._nameString + "[\"" + this._nameString + "\"]"; because the owner view in this case (a Scope) doesn't need to have an absolute name. @@ +3042,5 @@ > + if (this._symbolicName) { > + return this._symbolicName; > + } > + > + this._symbolicName = this.ownerView.symbolicName + "[\"" + this.name + "\"]"; Please use this._nameString here too.

Please address my comments first.

Thanks for the feedback! (In reply to Victor Porof [:vporof][:vp] from comment #41) > Why does a scope get an absolute name? It did not have one before. Only > variables and properties do. Please remove this. I've updated the patch, but why not? A scope may also have an absoluteName, it makes code more reliable (and cleaner), as I think.

Removed unnecessary comma

Comment on attachment 8459858 [details] [diff] [review] symbolicNamePerfv2.patch Review of attachment 8459858 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to railscard from comment #43) Loving the changes! > > Why does a scope get an absolute name? It did not have one before. Only > > variables and properties do. Please remove this. > > I've updated the patch, but why not? A scope may also have an absoluteName, > it makes code more reliable (and cleaner), as I think. Because we already have too many ways of accessing the name. Adding another one just makes things seem even more complicated.

(In reply to Victor Porof [:vporof][:vp] from comment #45) Okay, that makes sense. Thanks for your explanation! Can somebody run the tests, please?

(In reply to railscard from comment #46) > (In reply to Victor Porof [:vporof][:vp] from comment #45) > > Okay, that makes sense. > Thanks for your explanation! > > Can somebody run the tests, please? I'll push a new run momentarily.

Try: https://tbpl.mozilla.org/?tree=Try&rev=48a38c4a865a

Try looks good, so it's safe to set "checkin-needed".

railscard, thanks for your contribution! If you'd like to pick more DevTools bugs to work on, check out the suggested bug lists[1] or stop by the #devtools IRC room. Also, if you'd like to be able to run your own tests on Try server for future work, you can request level 1 commit access[2] and I'll vouch you. [1]: https://wiki.mozilla.org/DevTools/GetInvolved#Mentored_and_Good_First_Bugs [2]: https://www.mozilla.org/hacking/commit-access-policy/