Steps: 1. Open App Manager 2. Load an app 3. Remove a property from the app's manifest Expected result: The property is correctly removed from the manifest

Did you click "Save" after removing the property?

This is a tracking bug for adding a new automated test (removing a property) for the manifest editor.

Comment on attachment 8390559 [details] [diff] [review] remove property test Review of attachment 8390559 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the new test! Looks like just a few small tweaks to make. Be sure to run it on try as well. ::: browser/devtools/app-manager/test/browser_manifest_editor.js @@ +171,5 @@ > + let parentElem = gManifestWindow.document > + .querySelector("[id ^= '" + parent + "']"); > + ok(parentElem, "Found parent element"); > + > + if(parentElem == null) The block seems unnecessary, since you are testing value above. If it's null, the test fails anyway, which is what we want. @@ +178,5 @@ > + let keyExists = key in gManifestEditor.manifest[parent]; > + ok(keyExists, > + "The manifest contains the key under the expected parent"); > + > + if(!keyExists) Same here, remove this block. @@ +186,5 @@ > + let removePropertyButton = newElem.querySelector(".variables-view-delete"); > + ok(removePropertyButton, "The remove property button was found"); > + removePropertyButton.click(); > + > + waitForUpdate(); This should be "yield waitForUpdate()" as in the other functions above.

remote: https://tbpl.mozilla.org/?tree=Try&rev=b4e9a9752ec2

Comment on attachment 8391105 [details] [diff] [review] updated based on review Review of attachment 8391105 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks!