Closed
Bug 308916
Opened 19 years ago
Closed 18 years ago
Extension Manager ui string review / cleanup
Categories
(Toolkit :: Add-ons Manager, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.8.1beta1
People
(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)
References
Details
(Keywords: fixed1.8.1, late-l10n, Whiteboard: [AOMTestday])
Attachments
(3 files, 1 obsolete file)
(deleted),
image/png
|
beltzner
:
ui-review+
|
Details |
(deleted),
image/png
|
beltzner
:
ui-review+
|
Details |
(deleted),
patch
|
moco
:
review+
mconnor
:
approval1.8.1+
|
Details | Diff | Splinter Review |
There are several bugs floating around to change individual ui strings used by
the extension manager. Instead of fixing them individually this bug is for
reviewing all of them and updating them as appropriate. This will make it easier
to create consistent verbiage in the ui
See bug 303863, bug 303687, bug 302527, bug 292854, bug 290021, bug 275629, and
I'm sure others as well.
Updated•19 years ago
|
Assignee | ||
Comment 1•19 years ago
|
||
In relation to extension packaging errors I'd prefer to use a generic message
that also includes a statement to refer to the JavaScript Console for details.
My reasons off the top of my head are:
a) These errors should only be seen by extension authors.
b) when there is a regression in the EM we end up with bug reports regarding how
the detailed message is not appropriate to show to a user (see bug 302527 and
bug 292854).
c) simpler code for displaying these errors.
Comment 2•19 years ago
|
||
I'm a fan of the generic message for things like packaging errors, but not so
sure that it needs to have a specific call out to the JS console. Users need to
know that an error happened, and developers should know to look in the JS
console for more information.
Comment 3•19 years ago
|
||
Mike, that would be very newbie-developer unfriendly. It may sound weird, but
not every developer knows to look in the JavaScript console.
I think that since messages about packaging errors are seen by developers more
often than by users (ideally, users should never see it), the mention of JS
console is justified.
Comment 4•19 years ago
|
||
Well, we don't point people to the JS console when there are JS errors, nor for
CSS or HTML errors. I think the better approach is to train developers that they
can find detailed error messages in the JS console rather than repeat that fact
in error dialogs. But we can hash this out in the specific bugs :)
Assignee | ||
Comment 5•19 years ago
|
||
I don't see any harm with the message stating something along the lines of
"Please review the JavaScript Console for further information" for error
messages that only ext. developers would see with the exception of when there is
a regression that causes the message to be displayed. The fact is that many ext.
developers don't open the js console when they experience an error and this
would be a good way to train them since average users would seldom if ever see
these messages. As is we periodically get bug reports about the alert being too
generic, etc. from ext. developers when the details are already in the js console.
I believe that only one of the bugs this depends on relates to this type of
message and there are a couple of additional messages that fall into this
category... these probably haven't been reported since we haven't regressed in
the areas that would cause the message to be displayed whereas we did once in
the case of bug 302527.
Mike, do you have suggestions for a general packaging errors that only ext.
developers would see except in the case of a regression?
Comment 6•19 years ago
|
||
If you're saying that these are errors that only developers are likely to see, then something like:
"%productShortName was unable to load %extensionName. Please see the JavaScript console for details."
Might also be nice to have a checkbox option to "[ ] Open JavaScript Console" at the bottom, too.
Comment 7•19 years ago
|
||
Rob, some more EM UI string changes (these from the review requested in EM Dependencies, bug 298497)
Uninstall dialog
----------------
I like the changes to the button labels here, but the current text is a little redundant. I don't think people need to be informed that if they uninstall something, its function won't be available. Especially since we can't make any intelligent statements about what that function is, specifically. Maybe in the future we want to leave that as something that can be added by the extension itself. For now I'd suggest:
uninstallWarningMessage=This will remove the %S extension.
Similarly, the warning for installation with dependencies would become:
uninstallWarningDependMessage=%S is required by one or more extensions. If you continue, the following items will be disabled:
Missing Dependency message:
---------------------------
Any reason we're calling things "items" instead of extensions? "Operate" is also a slightly awkward term.
needsDependenciesDisabled=Disabled - missing one or more required extensions
Assignee | ||
Comment 8•19 years ago
|
||
Thanks Mike, the reason I used items is due to what we discussed regarding the use of the term extensions for things that aren't extensions as defined today. I am fine with using the term extensions for all items / addons (e.g. themes, plugins, searchplugins, locales, etc.) that are packaged for the Extension Manager.
Comment 9•19 years ago
|
||
Ah, then, go with whatever the generic term ends up being: extension, add-on, thingamajigger ...
Assignee | ||
Comment 10•19 years ago
|
||
Yo Mike, for the operation type messages I would like to go with the following which removes the redundant extension name which is displayed immediately above the message.
Will be upgraded when &brandShortName; is restarted
Will be installed when &brandShortName; is restarted
Will be uninstalled when &brandShortName; is restarted
Will be enabled when &brandShortName; is restarted
Will be disabled when &brandShortName; is restarted
r u down wit dat?
Assignee | ||
Updated•19 years ago
|
Assignee: nobody → robert.bugzilla
Target Milestone: --- → Firefox 2 beta1
Assignee | ||
Comment 11•18 years ago
|
||
I suspect that beltzner will end up doing the majority of the work on this one and it will be around 1 day of my time
Whiteboard: 1d
Updated•18 years ago
|
Flags: blocking-firefox2+
Assignee | ||
Comment 12•18 years ago
|
||
Bug 275629 - has the following strings and the bug is about displaying these strings from a local install.
itemWarningIntroMultiple=A web site is requesting permission to install the following %S items:
itemWarningIntroSingle=A web site is requesting permission to install the following item:
Not sure what we want for this one... I don't like the idea of having four strings to handle this. Perhaps making the strings more generic (e.g. remove "web site" from the strings).
Bug 302527 - this is an error that should only be seen by ext. authors or during nightly builds when there is a regression.
malformedRegistrationMessage=%S could not install this item because of a failure in Chrome Registration. Please contact the author about this problem.
Bug 292854 - is also an error that should only be seen by ext. authors. I'd like to remove the filename from the message since it is a temp file which has caused confusion.
%S could not install this item because "%S" (provided by the item) is not well-formed or does not exist. Please contact the author about this problem.
For Bug 302527 and Bug 292854 I'd like something along the lines of:
%S was unable install this item. Please contact the author about this problem (additional details are available in the JavaScript Console).
Comment 13•18 years ago
|
||
Added comments to dependencies: bug 275629, bug 292854, and bug 302527. Going against what I said in comment 6, I think we should still keep error messages short and sweet, and expect that developers will know/learn about the error console. I think that's quite likely the case, and want to avoid a needlessly tecchie experience on the off-chance an end user hits these warnings.
For this bug:
uninstallWarningMessage=This will remove the %S extension.
uninstallWarningDependMessage=%S is required by one or more extensions. If you
continue, the following items will be disabled:
needsDependenciesDisabled=Disabled - missing one or more required extensions.
> Yo Mike, for the operation type messages I would like to go with the following
> which removes the redundant extension name which is displayed immediately
> above the message.
> Will be upgraded when &brandShortName; is restarted
> Will be installed when &brandShortName; is restarted
> Will be uninstalled when &brandShortName; is restarted
> Will be enabled when &brandShortName; is restarted
> Will be disabled when &brandShortName; is restarted
>
> r u down wit dat?
Almost. Add a "This" and make it "will" and tack a "." on the end and we're golden. :)
(and ta-daah, I'm no longer blocking Rob!)
Comment 14•18 years ago
|
||
(In reply to comment #13)
> against what I said in comment 6, I think we should still keep error messages
> short and sweet, and expect that developers will know/learn about the error
> console. I think that's quite likely the case, and want to avoid a needlessly
> tecchie experience on the off-chance an end user hits these warnings.
I should mention, though, that if it's possible to add a "Open Error Console" button which closes the error message and opens the Error Console, that'd be acceptable. So:
.----------------------------------------------------------------.
| Oh noes! Something went wrong. It must be Rob Strong's Fault! |
| |
| ( Open Error Console ) ( OK ) |
'----------------------------------------------------------------'
- or -
| [ ] Open Error Console for more details |
| ( OK ) |
'----------------------------------------------------------------'
I suppose this could be done as a checkbox, but that just looks heavier and uglier to me.
Assignee | ||
Comment 15•18 years ago
|
||
(In reply to comment #14)
> .----------------------------------------------------------------.
> | Oh noes! Something went wrong. It must be Rob Strong's Fault! |
> | |
> | ( Open Error Console ) ( OK ) |
> '----------------------------------------------------------------'
If I can change that to "Robert Strong's" then I'm fine with that. :P
Benjamin, do all toolkit apps provide the error console?
(In reply to comment #13)
> uninstallWarningMessage=This will remove the %S extension.
This message is for extensions, themes, locales, and EM managed plug-ins. It also has a second line that states "Do you want to uninstall %s?" and having the "This will remove the %S extension." prior to that seems a bit redundant. How about just having the "Do you want to uninstall %s?"
> uninstallWarningDependMessage=%S is required by one or more extensions. If you
> continue, the following items will be disabled:
s/extensions/add-ons/
should items be changed to add-ons too? I could go either way.
> needsDependenciesDisabled=Disabled - missing one or more required extensions.
This is no longer used after the ui rewrite since now it can have multiple messages.
> > Yo Mike, for the operation type messages I would like to go with the following
> > which removes the redundant extension name which is displayed immediately
> > above the message.
> > Will be upgraded when &brandShortName; is restarted
> > Will be installed when &brandShortName; is restarted
> > Will be uninstalled when &brandShortName; is restarted
> > Will be enabled when &brandShortName; is restarted
> > Will be disabled when &brandShortName; is restarted
> >
> > r u down wit dat?
>
> Almost. Add a "This" and make it "will" and tack a "." on the end and we're
> golden. :)
I'm going to hold off on this on for a day to try to come up with something a little less awkward... any help would be appreciated.
Assignee | ||
Comment 16•18 years ago
|
||
beltzner, I am leaning towards "This add-on will be... etc." instead of "This will be... etc." since the latter seems a bit too Yoda-like.
Assignee | ||
Comment 17•18 years ago
|
||
beltzner, screenshots of the proposed changes to uninstall without the "This will remove the %S extension." text.
Attachment #225810 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 18•18 years ago
|
||
beltzner, screenshots of the action messages so you can get a feel for it. This will be just seems too awkward. :)
Attachment #225814 -
Flags: ui-review?(beltzner)
Assignee | ||
Comment 19•18 years ago
|
||
mconnor, this implements what is seen in the two attached images.
Attachment #225840 -
Flags: review?(mconnor)
Comment 20•18 years ago
|
||
Comment on attachment 225810 [details]
uninstall screenshot
Should there be a minimum width on the uninstall dialog? In the "Normal" case it looks a little unbalanced, like it could use some extra pixels on the right.
Attachment #225810 -
Flags: ui-review?(beltzner) → ui-review+
Comment 21•18 years ago
|
||
Comment on attachment 225814 [details]
Screenshot - action messages
Yes, you're right, the alternative text is better. Please use that!
Attachment #225814 -
Flags: ui-review?(beltzner) → ui-review+
Comment 22•18 years ago
|
||
(In reply to comment #15)
> (In reply to comment #14)
> > .----------------------------------------------------------------.
> > | Oh noes! Something went wrong. It must be Rob Strong's Fault! |
> > | |
> > | ( Open Error Console ) ( OK ) |
> > '----------------------------------------------------------------'
> If I can change that to "Robert Strong's" then I'm fine with that. :P
>
> Benjamin, do all toolkit apps provide the error console?
The other way to do this -- assuming that you're sent the full error message text and assuming that you can extend the error dialog to support it -- would be as shaver suggested:
.----------------------------------------------------------------.
| Oh noes! Something went wrong. It must be Rob Strong's Fault! |
| |
| ( Show Details ) ( OK ) |
'----------------------------------------------------------------'
- clicking details reveals .. -
| .---------------------------------------------------------. |
| | | |
| | <error text that can be copied and pasted> | |
| | | |
| | | |
| | | |
| '---------------------------------------------------------' |
| |
| ( Hide Details ) ( OK ) |
'----------------------------------------------------------------'
Assignee | ||
Comment 23•18 years ago
|
||
(In reply to comment #20)
> (From update of attachment 225810 [details] [edit])
> Should there be a minimum width on the uninstall dialog? In the "Normal" case
> it looks a little unbalanced, like it could use some extra pixels on the right.
I was thinking that there should be and will add it. Thanks.
Assignee | ||
Updated•18 years ago
|
Version: Trunk → 2.0 Branch
Assignee | ||
Comment 24•18 years ago
|
||
(In reply to comment #22)
To be honest I'm not that interested in adding this additional complexity to the EM for the small percentage of people that would find value in it. I think features like this belong in an extension developers kit which might be comprised of a set of extensions, possibly be a XULRunner app, or something else.
Assignee | ||
Updated•18 years ago
|
Whiteboard: 1d → [needs review]
Assignee | ||
Comment 25•18 years ago
|
||
Comment on attachment 225840 [details] [diff] [review]
patc
Benjamin, mconnor is a bit swamped with review requests... could you review this? Thanks
Attachment #225840 -
Flags: review?(mconnor) → review?(benjamin)
Assignee | ||
Comment 26•18 years ago
|
||
Seth, Benjamin is off today and this has late l10n changes... could you review this?
Attachment #226861 -
Flags: review?(sspitzer)
Comment 27•18 years ago
|
||
Comment on attachment 226861 [details] [diff] [review]
updated patch
r=sspitzer
do you need me to review that other patch to?
Attachment #226861 -
Flags: review?(sspitzer) → review+
Updated•18 years ago
|
Attachment #225840 -
Attachment is obsolete: true
Attachment #225840 -
Flags: review?(benjamin)
Assignee | ||
Comment 29•18 years ago
|
||
Checked in to trunk
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 30•18 years ago
|
||
Comment on attachment 226861 [details] [diff] [review]
updated patch
More late-l10n strings. :(
Attachment #226861 -
Flags: approval1.8.1?
Updated•18 years ago
|
Attachment #226861 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 31•18 years ago
|
||
Checked in to MOZILLA_1_8_BRANCH for Firefox 2.0
Keywords: fixed1.8.1
Updated•16 years ago
|
Product: Firefox → Toolkit
Comment 32•14 years ago
|
||
This bug seems to have been for 1.8.1, no need to keep it open any longer
Updated•14 years ago
|
Status: RESOLVED → VERIFIED
Whiteboard: [AOMTestday]
You need to log in
before you can comment on or make changes to this bug.
Description
•