205054 - [junk] need descriptive text dialog to come up for users 1st JMC action

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[esther]

Using trunk builds 20030508 on winxp, macosx and linux the user doesn't get the
descriptive text dialog with their first action of the Junk Mail Control
feature.  Per spec 
About Junk Mail - First Usage
   1. Junk Mail Controls are ON by default for all accounts. 
   2. Junk Mail Column ON by default.
   3. "Junk" toolbar button is visible by default.
   4. The first time the user does any of the actions listed below, the About
Junk Mail dialog opens.
          * Selects any of the JMC related menu items (see section A)
          * Clicks the "Junk" toolbar button
          * Turns on the Junk Mail Status column
   5. Clicking the "?" in the Junk Mail header in a message also opens the About
Junk Mail dialog.

#4 is not happening, I thought this was working earlier but is not working now.
   Possibly when we turned the Junk Mail Status column on by default this action
is treated as a users first action???  

1. Launch app
2. Create a New profile
3. Add a mail account and login 
4. Select the menu item Junk Mail Controls..  or any of the other JMC menu items
or the JUNK button on the toolbar.

Result: No descriptive dialog explaining JMC, the action happens instead.
Expected:  This was the 1st time use of JMC the user should get the descriptive
text dialog first.

Comment 1

[esther]

nominating, this was to be added to avoid user confusion of the feature per
usability study.   Note: the defaults for this feature that have been
implemented since the usability study have made this feature discoverable but
the dialog would be the finishing touch to explain it at first usage.  

Comment 2

[Samir Gehani]

adt: nsbeta1+/adt1

Comment 3

[Samir Gehani]

Over to Shuehan.

Comment 4

[shliang]

Attachment: patch

opens up info dialog the first time user does these
* Selects any of the JMC related menu items (see section A)
* Clicks the "Junk" toolbar button

not this one, because this is already on right?
* Turns on the Junk Mail Status column

Comment 5

[(not reading, please use [e-mail] instead)]

Comment on attachment 123299 [details] [diff] [review]
patch

needs some work.

1)  what if I close the info dialog by hitting the window [x]?

we don't need a callback here.	I suggest an onload, that sets the pref to
false.

2)

mail3PaneWindowCommands.js, what about the stand alone msg window?

I'd rather you fix (for example) MsgJunk() which is called by both 3 pane and
stand alone, to check the boolean.

I'm not sure we really need gJunkFirstUse either.  just check the pref each
time.  (I doubt this is a perf issue).

3)  you don't need to change MsgJunkMailInfo(), if you fix 1 and 2, I think.

4)

please also check
mGoodTokens.countTokens() > 0 and  mBadTokens.countTokens() > 0

Comment 6

[shliang]

Attachment: patch

Comment 7

[(not reading, please use [e-mail] instead)]

Comment on attachment 123344 [details] [diff] [review]
patch

1)

there's a problem.

MsgJunkMailInfo() is called from when the user hits the "?" in the junk mail
envelope.

but with your fix, that will stop working the first time.

also, instead of adding code to 
so do this:

function MsgJunkMailInfo(aCheckFirstUse)
{
  if (aCheckFirstUse) {
    if (!pref.getBoolPref("mailnews.ui.junk.firstuse"))
      return;

    pref.setBoolPref("mailnews.ui.junk.firstuse", false);

    // now check if they've been using junk mail
    // since we've had it since 1.3, we don't want experienced users to see
    // the info dialog
    var junkmailPlugin =
Components.classes["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"
]
				   
.getService(Components.interfaces.nsIJunkMailPlugin);
    if (junkmailPlugin.userClassified)
      return;
  }
  ...
}

now fix the callers of MsgJunkMailInfo() to be MsgJunkMailInfo(true), except
for the one behind "?"
MsgJunkMailInfo(false);

2)

+  *aResult = (mGoodCount > 0 && mGoodTokens.countTokens() > 0 &&
+	       mBadCount > 0 && mBadTokens.countTokens());

this isn't exactly right.  because you require both good and bad tokens
in order to avoid showing the info dialog, right?

also, why mBadTokens.countTokens(), but mGoodTokens.countTokens() > 0?	(not
consistent)

instead:

+  *aResult = ((mGoodCount && mGoodTokens.countTokens()) ||
+	       (mBadCount && mBadTokens.countTokens()));

Comment 8

[(not reading, please use [e-mail] instead)]

this should land during 1.4 final.

Comment 9

[shliang]

Attachment: patch

Comment 10

[(not reading, please use [e-mail] instead)]

Comment on attachment 123356 [details] [diff] [review]
patch

shuehan said she will change userClassified to userHasClassified and will add
back that comment about 1.3 users

so r/sr/a=sspitzer with those changes.

Comment 11

[(not reading, please use [e-mail] instead)]

Comment on attachment 123356 [details] [diff] [review]
patch

well, wait.

what about JunkSelectedMessages()?

and, clicking in the thread pane junk status icon?

Comment 12

[shliang]

Attachment: patch

Comment 13

[(not reading, please use [e-mail] instead)]

Comment on attachment 123416 [details] [diff] [review]
patch

1)

can you add a comment about why we are doing this?

+    var junkmailPlugin =
Components.classes["@mozilla.org/messenger/filter-plugin;1?name=bayesianfilter"
]
+				   
.getService(Components.interfaces.nsIJunkMailPlugin);
+    if (junkmailPlugin.userClassified)
+      return;
+  }

2)

have you re-tested?  junkmailPlugin.userClassified isn't going to work, and
should give you a js error.

+    readonly attribute boolean userHasClassified;

Comment 14

[shliang]

Attachment: patch

Comment 15

[(not reading, please use [e-mail] instead)]

shuehan sends me her onclick function, which makes it easier to read:

function ThreadPaneOnClick(event)
{
    // we only care about button 0 (left click) events
    if (event.button != 0) return;

    // we are already handling marking as read and flagging
    // in nsMsgDBView.cpp
    // so all we need to worry about here is double clicks
    // and column header.
    //
    // we get in here for clicks on the "treecol" (headers)
    // and the "scrollbarbutton" (scrollbar buttons)
    // we don't want those events to cause a "double click"

    var t = event.originalTarget;

    if (t.localName == "treecol") {
       HandleColumnClick(t.id);
    }
    else if (t.localName == "treechildren") {
      var row = new Object;
      var colID = new Object;
      var childElt = new Object;

      var tree = GetThreadTree();
      // figure out what cell the click was in
      tree.treeBoxObject.getCellAt(event.clientX, event.clientY, row, colID,
childElt);
      if (row.value == -1)
       return;

      // if the cell is in a "cycler" column
      // or if the user double clicked on the twisty,
      // don't open the message in a new window
      var col = document.getElementById(colID.value);
      if (col) {
        if (event.detail == 2 && col.getAttribute("cycler") != "true" &&
(childElt.value != "twisty")) {
          ThreadPaneDoubleClick();
          // double clicking should not toggle the open / close state of the 
          // thread.  this will happen if we don't prevent the event from
          // bubbling to the default handler in tree.xml
          event.preventBubble();
        }
        else if (colID.value == "junkStatusCol") {
          MsgJunkMailInfo(true);
        }
      }      
    }
}

Comment 16

[(not reading, please use [e-mail] instead)]

Comment on attachment 123429 [details] [diff] [review]
patch

r/sr/a=sspitzer

this should help new users, but not annoy existing users.

Comment 17

[shliang]

checked in

Comment 18

[esther]

Using trunk build 20030516 on winxp, macosx and linux this is fixed. Verified
Brought up when 1st:
clicking on any of the JMC menu items
when marking a message as junk 
when clicking on Junk button on a message analyzed as junk
when clicking on the junk button in toolbar

Comment 19

[esther]

found some specific scenarios that still fail to bring up the descriptive
dialog, logging new bug 206796.