Closed Bug 1017029 Opened 7 years ago Closed 7 years ago

[appmgr v2] Validation is not triggered after the file manifest has changed

Categories

(DevTools Graveyard :: WebIDE, defect)

x86_64
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 32

People

(Reporter: paul, Assigned: paul)

References

Details

Attachments

(1 file)

No description provided.
Attached patch v1Splinter Review
Small fix, big test.
Attachment #8430091 - Flags: review?(poirot.alex)
Blocks: build-am2
Comment on attachment 8430091 [details] [diff] [review]
v1

Review of attachment 8430091 [details] [diff] [review]:
-----------------------------------------------------------------

Can you ensure running this test on try before landing?
The previous try run you gave on bug 1007218 didn't seem to be running webide test:
  https://tbpl.mozilla.org/?tree=Try&rev=b27aaed038bc

::: browser/devtools/webide/test/test_manifestUpdate.html
@@ +35,5 @@
> +
> +            let project = win.AppManager.selectedProject;
> +
> +            let deferred = promise.defer();
> +            win.UI.projecteditor.once("onEditorCreated", deferred.resolve);

What about registering the listener before the call that is going to dispatch the event (importPackagedApp?)?
Is it Task-proof?

@@ +59,5 @@
> +            let json = JSON.parse(data);
> +            is(json.name, "xxx", "manifest written on disc");
> +
> +            // Make the manifest invalid on disk
> +            delete json.name

miss `;`
Attachment #8430091 - Flags: review?(poirot.alex) → review+
(In reply to Alexandre Poirot (:ochameau) from comment #2)
> Comment on attachment 8430091 [details] [diff] [review]
> v1
> 
> Review of attachment 8430091 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you ensure running this test on try before landing?
> The previous try run you gave on bug 1007218 didn't seem to be running
> webide test:
>   https://tbpl.mozilla.org/?tree=Try&rev=b27aaed038bc

I'll make sure all the tests pass in bug 1007057 (not required to land this patch).

> ::: browser/devtools/webide/test/test_manifestUpdate.html
> @@ +35,5 @@
> > +
> > +            let project = win.AppManager.selectedProject;
> > +
> > +            let deferred = promise.defer();
> > +            win.UI.projecteditor.once("onEditorCreated", deferred.resolve);
> 
> What about registering the listener before the call that is going to
> dispatch the event (importPackagedApp?)?
> Is it Task-proof?

projecteditor is not available earlier.

> @@ +59,5 @@
> > +            let json = JSON.parse(data);
> > +            is(json.name, "xxx", "manifest written on disc");
> > +
> > +            // Make the manifest invalid on disk
> > +            delete json.name
> 
> miss `;`

Ok.
Damn, I pushed and forgot to add the ';'. I don't want to push again just for this. I'll add the ';' in bug 1016924.
https://hg.mozilla.org/mozilla-central/rev/d9aa33eb16b6
Assignee: nobody → paul
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.