Closed Bug 912918 Opened 7 years ago Closed 6 years ago

[app manager] support for warning when there's errors

Categories

(DevTools :: WebIDE, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 28

People

(Reporter: paul, Assigned: jryans)

References

Details

Attachments

(3 files, 4 obsolete files)

Today, we don't show the warnings if there are errors.
Priority: -- → P2
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Hardware: x86 → All
Priority: P2 → P1
Comment on attachment 825073 [details] [diff] [review]
Display warnings and errors if both are present

I think there's more than that:

- warnings checks are not always run when there's an error (for example, use a manifest without a name and without an icon, we should have an error + a warning, but the warning check is not run because of a `return null` in app-validator.js)

- we should display warning messages if there are errors
Attachment #825073 - Flags: review?(paul) → review-
(In reply to Paul Rouget [:paul] from comment #2)
> Comment on attachment 825073 [details] [diff] [review]
> Display warnings and errors if both are present
> 
> I think there's more than that:
> 
> - warnings checks are not always run when there's an error (for example, use
> a manifest without a name and without an icon, we should have an error + a
> warning, but the warning check is not run because of a `return null` in
> app-validator.js)

Good call, I'll remove those cases so that validator will still proceed.

> - we should display warning messages if there are errors

That does indeed happen with this current patch. :P
Some small style cleanups that JSHint pointed out in the validator file.
Attachment #827094 - Flags: review?(paul)
This is the same patch you reviewed previously, just rebased and named "Part 2".

I was already displaying both warnings and errors if the validator stored both of them, so no changes were needed there.
Attachment #825073 - Attachment is obsolete: true
Attachment #827095 - Flags: review?(paul)
This allows the manifest to keep continue validating even if the name is missing.  

That's actually the only one I saw that can be changed...  All of the other "return null" paths are for real problems, like not having a manifest at all, or it has bad syntax.
Attachment #827098 - Flags: review?(paul)
Attachment #827094 - Flags: review?(paul) → review+
Attachment #827098 - Flags: review?(paul) → review+
Duplicate of this bug: 935263
Comment on attachment 827095 [details] [diff] [review]
Part 2: Display warnings and errors if both are present

> +  -moz-margin-start: 10px;

Why? We're not supposed to support bidi in projects.xhtml.
Attachment #827095 - Flags: review?(paul) → review+
Attachment #828675 - Attachment description: Code style cleanups in app validator v2 → Part 1: Code style cleanups in app validator v2
Carrying over Paul's r+ from attachment 827095 [details] [diff] [review].

(In reply to Paul Rouget [:paul] from comment #9)
> Comment on attachment 827095 [details] [diff] [review]
> Part 2: Display warnings and errors if both are present
> 
> > +  -moz-margin-start: 10px;
> 
> Why? We're not supposed to support bidi in projects.xhtml.

Good to know.  Changed to margin-left.
Attachment #827095 - Attachment is obsolete: true
Attachment #828735 - Flags: review+
Backed out for test_app_validator.html timeouts.
https://hg.mozilla.org/integration/fx-team/rev/8401f783b9f0

https://tbpl.mozilla.org/php/getParsedLog.php?id=30288914&tree=Fx-Team
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team] → [fixed-in-fx-te
Whiteboard: [fixed-in-fx-te
https://hg.mozilla.org/mozilla-central/rev/74303a630bea
https://hg.mozilla.org/mozilla-central/rev/b5bb24894892
https://hg.mozilla.org/mozilla-central/rev/9bfc039b9262
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.