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

RESOLVED FIXED in Firefox 28

Status

()

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

People

(Reporter: paul, Assigned: jryans)

Tracking

Trunk
Firefox 28
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

4 years ago
Today, we don't show the warnings if there are errors.
(Reporter)

Updated

4 years ago
Priority: -- → P2
(Assignee)

Updated

4 years ago
Blocks: 912912
(Assignee)

Updated

4 years ago
Assignee: nobody → jryans
Status: NEW → ASSIGNED
Hardware: x86 → All
(Reporter)

Updated

4 years ago
Priority: P2 → P1
(Assignee)

Comment 1

4 years ago
Created attachment 825073 [details] [diff] [review]
Display warnings and errors if both are present
Attachment #825073 - Flags: review?(paul)
(Reporter)

Comment 2

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

Comment 3

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

Comment 4

4 years ago
Created attachment 827094 [details] [diff] [review]
Part 1: Code style cleanups in app validator

Some small style cleanups that JSHint pointed out in the validator file.
Attachment #827094 - Flags: review?(paul)
(Assignee)

Comment 5

4 years ago
Created attachment 827095 [details] [diff] [review]
Part 2: Display warnings and errors if both are present

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)
(Assignee)

Comment 6

4 years ago
Created attachment 827098 [details] [diff] [review]
Part 3: Keep validating after errors when possible

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)
(Assignee)

Comment 7

4 years ago
Try: https://tbpl.mozilla.org/?tree=Try&rev=799a45eb43cb
(Reporter)

Updated

4 years ago
Attachment #827094 - Flags: review?(paul) → review+
(Reporter)

Updated

4 years ago
Attachment #827098 - Flags: review?(paul) → review+
(Reporter)

Updated

4 years ago
Duplicate of this bug: 935263
(Reporter)

Comment 9

4 years ago
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+
Created attachment 828675 [details] [diff] [review]
Part 1: Code style cleanups in app validator v2

Rebased, carrying over Paul's r+ from attachment 827094 [details] [diff] [review].
Attachment #827094 - Attachment is obsolete: true
Attachment #828675 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #828675 - Attachment description: Code style cleanups in app validator v2 → Part 1: Code style cleanups in app validator v2
Created attachment 828735 [details] [diff] [review]
Part 2: Display warnings and errors if both are present 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+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/763edcbd5757
https://hg.mozilla.org/integration/fx-team/rev/0b7c44eff352
https://hg.mozilla.org/integration/fx-team/rev/45b199e32b8e
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
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
Created attachment 828894 [details] [diff] [review]
Part 3: Keep validating after errors when possible v2

Carrying over Paul's r+ from attachment 827098 [details] [diff] [review].

Seems I forgot to add the test files.

Try: https://tbpl.mozilla.org/?tree=Try&rev=765f22fd590d
Attachment #827098 - Attachment is obsolete: true
Attachment #828894 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/74303a630bea
https://hg.mozilla.org/integration/fx-team/rev/b5bb24894892
https://hg.mozilla.org/integration/fx-team/rev/9bfc039b9262
Flags: in-testsuite+
Whiteboard: [fixed-in-fx-team]
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
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.