Comment on attachment 756209
fixed patch

Review of attachment 756209:
-----------------------------------------------------------------

Basically looks good, but i think we should skip warn on many for update
prompt. I suppose such cases exist in theory, but if it's an evil feed
people would unsubscribe pretty quickly once they notice there are 50000
new items in there. Unless the case is legit, and the prompt would be
just annoying.

Please also get ui-r from bwinton.

::: mail/locales/en-US/chrome/messenger-newsblog/newsblog.properties
@@ +55,5 @@
>  subscribe-confirmFeedDeletionTitle=Remove Feed
>  ## LOCALIZATION NOTE(subscribe-confirmFeedDeletion): %S is the name of the 
> feed the user wants to unsubscribe from.
>  subscribe-confirmFeedDeletion=Are you sure you want to unsubscribe from the 
> feed: \n %S?
>  
> +subscribe-confirmFeedTitle=Confirm Feed

"Subscribe to Feed" maybe? For one case. Probably should be different
string depending on if it's update or not. If we'd keep the update case.

@@ +57,5 @@
>  subscribe-confirmFeedDeletion=Are you sure you want to unsubscribe from the 
> feed: \n %S?
>  
> +subscribe-confirmFeedTitle=Confirm Feed
> +## LOCALIZATION NOTE(subscribe-confirmSubscribe): %S is the number of items 
> in a new feed.
> +subscribe-confirmSubscribe=This feed contains %S items. Continue?

I think this too needs more clarification.  Maybe something like
"This feed contains a lot of items (%S) and may therefore be slow. Are you sure 
you want to subscribe?"

And make the button labeled &Go on instead?

::: mailnews/extensions/newsblog/content/feed-parser.js
@@ +19,5 @@
> +    if (!(aDOM instanceof Ci.nsIDOMXMLDocument))
> +    {
> +      // No xml doc.
> +      aFeed.onParseError(aFeed);
> +      return new Array();

return []; is shorter

::: mailnews/extensions/newsblog/content/utils.js
@@ +699,5 @@
> +    }
> +
> +    if (!Services.prompt.confirmEx(null, pTitle, pMessage,
> +                                   Ci.nsIPromptService.STD_OK_CANCEL_BUTTONS,
> +                                   null, null, null, null, { }))

please use verbs for the action buttons instead of ok/cancel

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/191006

Title:
  Thunderbird RSS should have a per feed limit per download

To manage notifications about this bug go to:
https://bugs.launchpad.net/thunderbird/+bug/191006/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to