Closed
Bug 949981
Opened 10 years ago
Closed 10 years ago
collection of lockPref setting bugs (some structured options in preferences dialog are not disabled properly when the associated pref is locked)
Categories
(Thunderbird :: Preferences, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 29.0
People
(Reporter: spam2, Assigned: aceman)
References
(Blocks 1 open bug)
Details
(Keywords: conversion)
Attachments
(2 files, 3 obsolete files)
192.39 KB,
video/avi
|
Details | |
4.54 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release) Build ID: 20131205075310 Steps to reproduce: Insert the following lines into mozilla.cfg lockPref("mailnews.mark_message_read.auto", true); lockPref("mailnews.mark_message_read.delay", false); Actual results: The corresponding radiogroup id="markAsReadAutoPreferences" is still enabled. Only if you switch the radiobutton manually to the other option using the preferences gui the radiogroup gets disabled. Expected results: The corresponding radiogroup id="markAsReadAutoPreferences" should be grayed out and get disabled for user changes
Where should I put the mozilla.cfg file to try this out?
Assignee: nobody → acelists
At first you have to create a 'local-settings.js' file in the 'defaults\pref' subfolder of your thunderbird installation directory, e.g. '%ProgramFiles(x86)%\Mozilla Thunderbird\defaults\pref' This 'local-settings.js' files must contain the following two lines: pref("general.config.obscure_value", 0); pref("general.config.filename", "mozilla.cfg"); After this you can create the 'mozilla.cfg' file in the Thunderbird installation directory, e.g. '%ProgramFiles(x86)%\Mozilla Thunderbird' In the 'mozilla.cfg' file you have to put the following three lines: // lockPref("mailnews.mark_message_read.auto", true); lockPref("mailnews.mark_message_read.delay", false); It is important, that the first line in 'mozilla.cfg' file contains '//' as shown above. Now you can run Thunderbird and try this out.
Reporter, are you able to try out the patch?
Attachment #8347495 -
Flags: review?(mkmelin+mozilla)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Unfortunately I am not able to compile Thunderbird on my own. Is the attached diff belonging to source files in the thunderbird installation directory, that don't have to be compiled?
Yes, the change is in Javascript so you can find the file advanced.js in the omni.ja archive (a zip archive), do the changes manually and repack the archive. Then delete the startupCache folder in your TB profile.
Thanks for the patch. But I am not able to apply it. It seems that the advanced.js you used for your changes is different from the one that resists in omni.ja archive for Thunderbird 24.2.0. In my advanced.js file the function updateMarkAsReadOptions starts at line 300, whereas in your patch the same function starts at line 303. That's a bit confusing. Futher more it would be much easier for me if you could attach a complete advanced.js and advanced.xul, because I don't have any program installed to apply a diff to a source file automatically. I always have to use copy and paste :-( Sorry for that.
Meanwhile I did apply your proposed changes manually. Good news: It's working now. Bad news: There's one more bug left in this radiogroup If you insert the following three lines in mozilla.cfg, only the textbox 'markAsReadDelay' should be disabled. // lockPref("mailnews.mark_message_read.auto", true); // lockPref("mailnews.mark_message_read.delay", false); lockPref("mailnews.mark_message_read.delay.interval", 0); But that's not the case. Since the other lines are commented out, the setting 'mailnews.mark_message_read.delay' defaults to 'false' or it is set to a value specified by the user before. If you switch the radio button manually in the GUI to 'true', the textbox 'markAsReadDelay' gets enabled again. If you klick on one of the spinbuttons next to this textbox, or if you manually change this value through typing, the textbox will be disabled again after having applied the change. The right behaviour should be, that if you specify a lockPref setting for 'mailnews.mark_message_read.delay.interval', the corresponding textbox has to be always disabled, no matter if you switch the radiobuttons in the group 'markAsReadAutoPreferences'.
Perhaps we should rename the headline for this bug report: 'lockPref setting bugs' Here's another one: lockPref("mail.purge_threshhold_mb", 50); Specifying this setting, the corresponding textbox 'offlineCompactFolderMin' should be disabled
(In reply to egal8888 from comment #7) > If you insert the following three lines in mozilla.cfg, only the textbox > 'markAsReadDelay' should be disabled. Yes, I have noticed that too. In the code you can see: + let intervalPref = document.getElementById("mailnews.mark_message_read.delay.interval"); document.getElementById('markAsReadDelay').disabled = + !enableRadioGroup || !autoMarkAsPref.value || autoMarkAsPref.locked || + intervalPref.locked; So, I try to disable 'markAsReadDelay' if the "mailnews.mark_message_read.delay.interval" pref is locked. I do not understand why the textbox looks enabled but disables itself when clicked any input is attempted. But at least the user can't change the value. May this be a bug of the widget?
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to egal8888 from comment #8) > Perhaps we should rename the headline for this bug report: > 'lockPref setting bugs' > > Here's another one: > > lockPref("mail.purge_threshhold_mb", 50); > > Specifying this setting, the corresponding textbox 'offlineCompactFolderMin' > should be disabled Yes, I think there are more fields where this checking is missing. If you are willing to test all of them then I can fix them.
Reporter | ||
Comment 11•10 years ago
|
||
(In reply to :aceman from comment #10) > Yes, I think there are more fields where this checking is missing. If you > are willing to test all of them then I can fix them. At the moment I'm trying my best to roll out thunderbird to our company. That's not as easy as I thought it would be. So, please leave this bug opened until I set the status to "FIXED", since I have not tested all settings yet. If I run into other bugs concerning lockPref settings, I will post them here, ok?
Summary: lockPref for mailnews.mark_message_read not working as expected → collection of lockPref setting bugs
Comment 12•10 years ago
|
||
(In reply to aceman from comment #9) > So, I try to disable 'markAsReadDelay' if the > "mailnews.mark_message_read.delay.interval" pref is locked. I do not > understand why the textbox looks enabled but disables itself when clicked > any input is attempted. But at least the user can't change the value. > > May this be a bug of the widget? Sorry, I'm unclear as to what the problem is here. Does the textbox disable correctly if you switch the radiobutton?
Flags: needinfo?(neil)
Reporter | ||
Comment 13•10 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #12) > (In reply to aceman from comment #9) > Sorry, I'm unclear as to what the problem is here. Does the textbox disable > correctly if you switch the radiobutton? No, it doesn't. If you switch the radiobutton the textbox is enabled again. Only in case a user tries to make a change to the value in this textbox, this box becomes disabled again.
Reporter | ||
Comment 14•10 years ago
|
||
This video shows how Thundebird behaves when the radio button is switched.
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to egal8888 from comment #11) > So, please leave this bug opened until I set the status to "FIXED", since I > have not tested all settings yet. > If I run into other bugs concerning lockPref settings, I will post them > here, ok? Yes, we can try it. We can check in partial patches. If this proves to be too long (time-wise) we can close this bug and open a new one for new non-working fields.
Assignee | ||
Comment 16•10 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #12) > Sorry, I'm unclear as to what the problem is here. Does the textbox disable > correctly if you switch the radiobutton? The video shows the problem. We set .disabled on the textbox, however it does not appear disabled (it looks editable). As soon as you click the increment/decrement arrows or try to write into the textbox, it disables itself (without any code on our side).
Summary: collection of lockPref setting bugs → collection of lockPref setting bugs (some structured options in preferences dialog are not disabled properly when the associated pref is locked)
Assignee | ||
Comment 17•10 years ago
|
||
Oh, scratch it, false alarm. There actually is code attached to the preference that I didn't notice: <preference id="mailnews.mark_message_read.delay" name="mailnews.mark_message_read.delay" type="bool" onchange="gAdvancedPane.updateMarkAsReadTextbox(this.value);"/> updateMarkAsReadTextbox: function(aFocusTextBox) { var textbox = document.getElementById('markAsReadDelay'); textbox.disabled = !document.getElementById('markAsReadAfterDelay').selected; if (!textbox.disabled && aFocusTextBox) textbox.focus(); },
Assignee | ||
Comment 18•10 years ago
|
||
OK, this patch fixes the problem and also fixes the textbox from comment 8. Can you try this full advanced.js ?
Attachment #8347495 -
Attachment is obsolete: true
Attachment #8347495 -
Flags: review?(mkmelin+mozilla)
Attachment #8348282 -
Flags: feedback?(spam2)
Reporter | ||
Comment 19•10 years ago
|
||
Wow, thanks a lot. All previous described bugs are fixed now. But... the changes to advanced.js indroduce a new issue: If you set lockPref("app.update.auto", false) the whole radiogroup 'updateMode' including checkbox 'warnIncompatible' has to be disabled. With your patched advanced.js this is not the case. The checkbox 'warnIncompatible' stays enabled. The original advanced.js from Thunderbird release 24.2.0 does not have this issue. Could you please have a look at this?
Assignee | ||
Comment 20•10 years ago
|
||
Yeah, sorry, I attached the raw source of the file. It first needs to be preprocessed for each platform to remove unwanted code. Didn't you get errors in the Error console when running with this raw file? I do not think I have touched anything for update handling, therefore I think there may be parse errors so some code was not run properly. For Windows, I think you need to remove all lines starting with #. NOT whole blocks inside of #ifdef, just the lines as the code inside the blocks is applicable on Windows.
Reporter | ||
Comment 21•10 years ago
|
||
(In reply to :aceman from comment #20) > Didn't you get errors in the Error console when running with this raw file? No, I did not get errors in the console. > For Windows, I think you need to remove all lines starting with #. NOT whole > blocks inside of #ifdef, just the lines as the code inside the blocks is > applicable on Windows. I removed all lines starting with # and yes, I can confirm that it is working now. In which future version of Thunderbird will your patch be included? I did not find any other issues with lockPref yet, but I have also not yet tested every option. As this is my last week on work for this year :-) I think I will not finish testing in 2013.
Assignee | ||
Comment 22•10 years ago
|
||
(In reply to egal8888 from comment #21) > In which future version of Thunderbird will your patch be included? It should be in TB29, but if you ask about official stable releases then it is TB31, due out in about 30 weeks. If you need it sooner for your enterprise, you can provide reasons why it is important and the release managers will decide whether to include it in a sooner maintenance release of TB24.x . If they see it is a low risk small patch. I've linked this bug to our meta bug to track enterprise problems. > I did not find any other issues with lockPref yet, but I have also not yet > tested every option. > As this is my last week on work for this year :-) I think I will not finish > testing in 2013. No, problem so we can then close this bug and if you find anything new in future you can open a new one. But now before the big plans I need to attach the patch for review :)
Blocks: tb-enterprise
Keywords: conversion
Assignee | ||
Comment 23•10 years ago
|
||
Attachment #8348282 -
Attachment is obsolete: true
Attachment #8348282 -
Flags: feedback?(spam2)
Attachment #8350751 -
Flags: review?(mkmelin+mozilla)
Comment 24•10 years ago
|
||
Comment on attachment 8350751 [details] [diff] [review] patch Review of attachment 8350751 [details] [diff] [review]: ----------------------------------------------------------------- r=mkmelin ::: mail/components/preferences/advanced.js @@ +305,5 @@ > + * state of the automatic marking feature. > + * > + * @param enableRadioGroup Boolean value indicating whether the feature is enabled. > + */ > + updateMarkAsReadOptions: function(aEnableRadioGroup) @param aEnableRadioGroup
Attachment #8350751 -
Flags: review?(mkmelin+mozilla) → review+
Assignee | ||
Comment 25•10 years ago
|
||
Thanks.
Attachment #8350751 -
Attachment is obsolete: true
Attachment #8355623 -
Flags: review+
Keywords: checkin-needed
Comment 26•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/25da80686360
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in
before you can comment on or make changes to this bug.
Description
•