Last Comment Bug 525905 - Attachment reminder keywords list opens wrong options tab
: Attachment reminder keywords list opens wrong options tab
: polish
Product: Thunderbird
Classification: Client Software
Component: Message Compose Window (show other bugs)
: Trunk
: All All
-- normal (vote)
: Thunderbird 17.0
Assigned To: :aceman
Depends on: 360891 718139 761797
  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:
QA Whiteboard:
Iteration: ---
Points: ---

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

Description User image 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 User image Thomas D. (currently busy elsewhere; needinfo?me) 2010-01-22 13:29:28 PST

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

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

- ensure that focus is on composition > general tab, where the button for editing the keyword list is located
Comment 2 User image 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 User image :aceman 2012-05-29 23:00:23 PDT
The call seems to be here, it only specifies the pane.

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 User image :aceman 2012-07-17 15:14:26 PDT
Created attachment 643159 [details] [diff] [review]

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 User image Blake Winton (:bwinton) (:☕️) 2012-07-23 12:12:10 PDT
Comment on attachment 643159 [details] [diff] [review]

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!  :)

Comment 6 User image 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 User image :aceman 2012-07-23 13:17:31 PDT
OK, can you see if this patch: from bug 360891 makes the dialogs appear better?
Comment 8 User image Magnus Melin 2012-07-28 11:19:35 PDT
Comment on attachment 643159 [details] [diff] [review]

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 User image :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 User image :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 User image 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 User image Ryan VanderMeulen [:RyanVM] 2012-08-25 13:00:40 PDT

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