We need to use l10n_client on a long term basis on some project. Users that can access l10n_client have by far not admin permissions (to enable/disable the module or remove permission settings). Also, enabling the tool should only be a private action. Not for the whole admin role members.
We'd like to see a user setting to enable / disable l10n_client. Thus we can remain flexible and still have a system with higher performance when disabling the client. In hook_user there's already some server settings which can be extended very simple.
Since we're having multiple patches in our system (multiple text groups, ...) we're adding some snippets here.
function l10n_client_init() {
global $conf, $language, $user;
if (user_access('use on-page translation') && $user->l10n_client_enable) {
...
function l10n_client_footer() {
global $conf, $language, $user;
// Check permission and get all strings used on the page.
if (user_access('use on-page translation') && $user->l10n_client_enable
...
function l10n_client_user($type, $edit, &$account, $category = NULL) {
if ($type == 'form' && $category == 'account') {
if ((variable_get('l10n_client_use_server', FALSE) && user_access('submit translations
to localization server', $account))
|| user_access('use on-page translation')) {
$form['l10n_client'] = array(
'#type' => 'fieldset',
'#title' => t('Localization client'),
'#weight' => 1,
);
}
...
if (user_access('use on-page translation')) {
$form['l10n_client']['l10n_client_enable'] = array(
'#type' => 'checkbox',
'#title' => t('Enable on-page translation'),
'#default_value' => !empty($edit['l10n_client_enable']) ? $edit['l10n_client_enable'] : FALSE,
'#description' => t('Use the localization client on the bottom of each page to edit localization data.'),
);
}
}
Generally that's it, perfectly running overhere.
We'll provide patch as soon as you confirm this to be included.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | d7peruser.patch | 5.43 KB | gábor hojtsy |
| #13 | user-setting.patch | 5.98 KB | GH |
| #12 | l10n_client-774794_1.patch | 6.23 KB | dawehner |
| #10 | l10n_client-774794_1.patch | 6.23 KB | dawehner |
| #5 | l10n_client-774794.patch | 4.91 KB | dawehner |
Comments
Comment #1
dawehnerThis is a patch for exxct this code.
This code looks fine.
Comment #2
dawehnerThis needs work.
Comment #3
dawehnerSo total rewrite:
* A new access function l10n_client_access, which will be used on several places over the module, also on the hook_menu
* per default, it should be enabled.
Comment #4
miro_dietikerAt least typo "acccount". Everything else looks fine. (still untested)
Gabor, will we get your support for this piece of work?
Comment #5
dawehnerThanks. I bet noone would have used this functionality, but this is a common pattern for access functions, so i implemented it here.
Comment #6
dawehnerThis would need a rerole for 7.x, but it should be doable.
Comment #7
gábor hojtsyGreat idea, I support this new feature! Found a couple of issues by looking at the code:
- I'd name the user setting l10n_client_disabled (not "enable"), since that would let us just use !empty($user->l10n_client_disabled) instead of special casing TRUE if the setting did not exist.
- the user_access('use on-page translation') in the hook_user() does not have an $account argument
- the server API key field is output with the patch even if there is no server setup
- "ot make the translation" typo
Let's get these fixed, committed to D6 and ported to D7 then!
Comment #8
dawehnerI thought of disabled instead of enabled, too. It makes much more sense here.
The rest of the issues are fixed, too.
Comment #9
gábor hojtsy@dereine: missed to upload the patch?
Comment #10
dawehnerah forgot the cvs diff, but i made a cvs update -CdPR between, damn.
So another try.
Comment #11
miro_dietikerTypo in !empty($account->l10n_clien_disable)
...client...
All other things look fine. Being eager to do the "final" cvs update and drop our customizations. ;-)
Comment #12
dawehnerhere is the patch
Comment #13
gábor hojtsyOk, applied this and fixed a few issues:
- let's name it l10n_client_disabled instead of disable
- the access condition was not right, !empty(disabled) was a required condition for access, when the contrary should happen
- the conditions for the form elements were duplicated in hook_user... I've modified this to not repeat that, but instead look if there were form elements added and then wrap them in a fieldset (makes it easier to maintain)
- modified the text for the checkbox and made it compatible with the soon to be added admin 6.x-2.x compatibility code (ie. the bar will not necessarily be at the bottom)
- made it use $account instead of $edit just like the other setting
This now looks good IMHO, so let's get another round of reviews before getting in.
Comment #14
hass commented+
Comment #15
miro_dietikerPerfectly works as live checkout 6--1 applied with your patch on our testing server and finally on our live site.
Thanks a lot for all your support! Please apply.
Comment #16
gábor hojtsyPerfect, committed to Drupal 6, thanks! Needs D7 port.
Comment #17
gábor hojtsyHere is the D7 port, committed.