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

RESOLVED FIXED in Firefox 32

Status

()

Firefox
Developer Tools: WebIDE
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: paul, Assigned: paul)

Tracking

Trunk
Firefox 32
x86_64
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

4 years ago
Created attachment 8430091 [details] [diff] [review]
v1

Small fix, big test.
Attachment #8430091 - Flags: review?(poirot.alex)
(Assignee)

Updated

4 years ago
Blocks: 1007057
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+
(Assignee)

Comment 3

4 years ago
(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.
(Assignee)

Comment 4

4 years ago
https://hg.mozilla.org/integration/fx-team/rev/d9aa33eb16b6
Whiteboard: [fixed-in-fx-team]
(Assignee)

Comment 5

4 years ago
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.

Comment 6

4 years ago
https://hg.mozilla.org/mozilla-central/rev/d9aa33eb16b6
Assignee: nobody → paul
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 32
You need to log in before you can comment on or make changes to this bug.