Last Comment Bug 525905 - Attachment reminder keywords list opens wrong options tab
: Attachment reminder keywords list opens wrong options tab
Status: RESOLVED FIXED
: polish
Product: Thunderbird
Classification: Client Software
Component: Message Compose Window (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: :aceman
:
Mentors:
Depends on: 360891 718139 761797
Blocks:
  Show dependency treegraph
 
Reported: 2009-11-02 10:12 PST by Stefan Plewako [:stef]
Modified: 2012-08-25 13:00 PDT (History)
7 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (4.02 KB, patch)
2012-07-17 15:14 PDT, :aceman
mkmelin+mozilla: review+
bwinton: ui‑review+
Details | Diff | Review
patch v2 (2.94 KB, patch)
2012-08-16 13:42 PDT, :aceman
mkmelin+mozilla: review+
Details | Diff | Review

Description Stefan Plewako [:stef] 2009-11-02 10:12:28 PST
When attachment reminder bar opens and one will click on keyword list, the settings window will open on "Composition" but with wrong tab selected if other then the "General" tab was the last open one.
Comment 1 Thomas D. (currently busy elsewhere; needinfo?me) 2010-01-22 13:29:28 PST
STR

1 tools > options > composition: set focus on spelling tab, close options dialogue with ok or cancel
2 write mail with attachment keyword
-> attachment reminder bar pops up (ok)
3 click on linkified keyword to edit list of attachment reminder keywords

actual
- options window opens with focus still on spelling tab
- can't edit keywords list from here
-> confusion for users

expected
- ensure that focus is on composition > general tab, where the button for editing the keyword list is located
Comment 2 Thomas D. (currently busy elsewhere; needinfo?me) 2012-05-28 23:17:52 PDT
Bug 718139 Comment 13 experiences the same problem as this bug.
Comment 3 :aceman 2012-05-29 23:00:23 PDT
The call seems to be here, it only specifies the pane.
http://mxr.mozilla.org/comm-central/source/mail/components/compose/content/MsgComposeCommands.js#1934

If infrastructure for opening specific pane and tab lands in bug 718139, then this should be made to work easily.
However I wonder why selecting the proper pane (composition) already works... I didn't find the code for that and re-implement it in bug 718139.
Comment 4 :aceman 2012-07-17 15:14:26 PDT
Created attachment 643159 [details] [diff] [review]
patch

I can even open just the attachment keyword editor hiding the full preferences dialog. The patch uses the same approach as bug 360891. Think bwinton had some problem with that on Mac. Magnus Melin, maybe you can look at it too, if you want the full prefs dialog or only the keyword editor (subdialog).
Also this patch must be applied on top of patch from bug 761797 as it changes the same file.
Comment 5 Blake Winton (:bwinton) (:☕️) 2012-07-23 12:12:10 PDT
Comment on attachment 643159 [details] [diff] [review]
patch

So, I'm almost really happy with it…  ;)

The only problem I have is that the attachment keywords show up before the preference dialog does, and it feels really strange.

When you fix that, ui-r=me!  :)

Thanks,
Blake.
Comment 6 Blake Winton (:bwinton) (:☕️) 2012-07-23 12:12:49 PDT
(Uh, that's on Mac, since it seems like it would make a difference.)
Comment 7 :aceman 2012-07-23 13:17:31 PDT
OK, can you see if this patch: https://bugzilla.mozilla.org/attachment.cgi?id=644488 from bug 360891 makes the dialogs appear better?
Comment 8 Magnus Melin 2012-07-28 11:19:35 PDT
Comment on attachment 643159 [details] [diff] [review]
patch

Review of attachment 643159 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me, i do like just showing the dialog with the keywords. Sorry for the delay. r=mkmelin

::: mail/components/compose/content/MsgComposeCommands.js
@@ +1901,5 @@
>      msg.setAttribute("flex", "100");
>  
>      msg.onclick = function(event)
>      {
> +      openOptionsDialog("paneCompose", "generalTab", {subdialog_only: "attachment_reminder_button"});

overlong line, wrap after generaltab
Comment 9 :aceman 2012-07-28 11:38:56 PDT
Thanks. Unfortunately bwinton rejected this approach in bug 360891 and wants to see the preferences dialog too. The same in comment 5 here. It probably flickers too much on some setups.

So I need to rework the patch.
Comment 10 :aceman 2012-08-16 13:42:16 PDT
Created attachment 652550 [details] [diff] [review]
patch v2

Reworked per bwinton's preference. The advantage of this solution is that the user also sees the "check for missing attachment" option, which wouldn't be the case if only the subdialog was shown.
Comment 11 Magnus Melin 2012-08-18 12:05:17 PDT
Comment on attachment 652550 [details] [diff] [review]
patch v2

Review of attachment 652550 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me! r=mkmelin
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-08-25 13:00:40 PDT
https://hg.mozilla.org/comm-central/rev/5322ef0f9d8e

Note You need to log in before you can comment on or make changes to this bug.