349606 - Simplify "Add Feed Reader" UI

Status: RESOLVED FIXED

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[Jeff Walden [:Waldo]]

What in the world do OK/Cancel mean in this dialog?  I have no idea.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: The dialog

Comment 2

[Mike Connor [:mconnor]]

easy plan: 
hide the site line
hide the checkbox
hide the Cancel button

harder plan:
figure this out in the caller and open an alert instead of this (assuming that its a user-event, otherwise do nothing)

easy plan, due to how this code seems to work, is the faster/safer plan, I think.

Comment 3

[Mike Connor [:mconnor]]

So, what we need to do here is handle the following questions:

- How to treat the "always use X pref"
- What to show, given string-frozenness

in cases where

- The engine is installed, and is set to always be used
- The engine is installed, and another engine is set to always be used
- The engine is installed, and we are set to always ask

Comment 4

[Asaf Romano (gone)]

Here's my current plan:
  - remove the checkbox altogether.
  - make "no" the default button.
  - if the user chooses to add the reader:
    * Go back to "Show Preview" mode.
    * make the new reader the last-selected reader (meaning it will be selected the next time you see the feed subscription UI).

Comment 5

[Mike Connor [:mconnor]]

Sounds great, let's get it done :)

Comment 6

[Asaf Romano (gone)]

Attachment: patch (branch only)

Comment 7

[Mike Connor [:mconnor]]

Comment on attachment 235682 [details] [diff] [review]
patch (branch only)

>+    // We don't need to add an already-installed handelr
>+    this._result.SetInt(PARAM_SHOULD_ADD_HANDLER, !this._handlerInstalled);

s/handelr/handler/

>   /**
>    * See nsIWebContentHandlerRegistrar
>    */
>   registerProtocolHandler: 
>-  function WCCR_registerProtocolHandler(protocol, uri, title, contentWindow) {
>+  function WCCR_registerProtocolHandler(protocol, uriString, title, contentWindow) {
>     // XXXben - for Firefox 2 we only support feed types
>     return;
>-    
>+
>+    try {
>+      var uri = this._makeURI(uriString);
>+    }
>+    catch(ex) { return; }
>+
>     LOG("registerProtocolHandler(" + protocol + "," + uri + "," + title + ")");
>     if (this._confirmAddHandler(protocol, title, uri, contentWindow))
>       this._protocols[protocol] = uri;
>   },

if we already preprocess this file, we should #ifdef 0 around the unused code, otherwise leave as-is

r=me with those nits

a=me on behalf of drivers for this branch-only patch, discussed with beltzner

Comment 8

[Asaf Romano (gone)]

1.8 branch:
mozilla/browser/components/feeds/content/addFeedReader.js 1.1.2.4
mozilla/browser/components/feeds/content/addFeedReader.xul 1.1.2.3
mozilla/browser/components/feeds/src/WebContentConverter.js 1.1.2.14

-> fixed. trunk work tbd on bug 349380.

Comment 9

[pal-moz]

need UI in Options>Feeds. 
bug350133