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

VERIFIED FIXED in mozilla1.4final

Status

MailNews Core
Backend
VERIFIED FIXED
15 years ago
10 years ago

People

(Reporter: esther, Assigned: shliang)

Tracking

Trunk
mozilla1.4final

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt1])

Attachments

(1 attachment, 4 obsolete attachments)

8.79 KB, patch
(not reading, please use seth@sspitzer.org instead)
: review+
(not reading, please use seth@sspitzer.org instead)
: superreview+
(not reading, please use seth@sspitzer.org instead)
: approval1.4+
Details | Diff | Splinter Review
(Reporter)

Description

15 years ago
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.
(Reporter)

Comment 1

15 years ago
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.  
Keywords: nsbeta1

Comment 2

15 years ago
adt: nsbeta1+/adt1
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt1]

Comment 3

15 years ago
Over to Shuehan.
Assignee: sspitzer → shliang
(Assignee)

Comment 4

15 years ago
Created attachment 123299 [details] [diff] [review]
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
(Assignee)

Updated

15 years ago
Attachment #123299 - Flags: review?(sspitzer)
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
Attachment #123299 - Flags: review?(sspitzer) → review-
(Assignee)

Comment 6

15 years ago
Created attachment 123344 [details] [diff] [review]
patch
Attachment #123299 - Attachment is obsolete: true
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()));
Attachment #123344 - Flags: review-
this should land during 1.4 final.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.4final
(Assignee)

Comment 9

15 years ago
Created attachment 123356 [details] [diff] [review]
patch
Attachment #123344 - Attachment is obsolete: true
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.
Attachment #123356 - Flags: superreview+
Attachment #123356 - Flags: review+
Attachment #123356 - Flags: approval1.4+
Comment on attachment 123356 [details] [diff] [review]
patch

well, wait.

what about JunkSelectedMessages()?

and, clicking in the thread pane junk status icon?
(Assignee)

Comment 12

15 years ago
Created attachment 123416 [details] [diff] [review]
patch
Attachment #123356 - Attachment is obsolete: true
Attachment #123356 - Flags: superreview+
Attachment #123356 - Flags: review+
Attachment #123356 - Flags: approval1.4+
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;
Attachment #123416 - Flags: review-
(Assignee)

Comment 14

15 years ago
Created attachment 123429 [details] [diff] [review]
patch
Attachment #123416 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #123429 - Flags: review?(sspitzer)
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 on attachment 123429 [details] [diff] [review]
patch

r/sr/a=sspitzer

this should help new users, but not annoy existing users.
Attachment #123429 - Flags: superreview+
Attachment #123429 - Flags: review?(sspitzer)
Attachment #123429 - Flags: review+
Attachment #123429 - Flags: approval1.4+
(Assignee)

Comment 17

15 years ago
checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 18

15 years ago
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
Status: RESOLVED → VERIFIED
(Reporter)

Comment 19

15 years ago
found some specific scenarios that still fail to bring up the descriptive
dialog, logging new bug 206796.
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.