Re: [SM-USERS] Curious use of "use_js_ in addresbook templates

View: New views
4 Messages — Rating Filter:   Alert me  

Parent Message unknown Re: [SM-USERS] Curious use of "use_js_ in addresbook templates

by Paul Lesniewski :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

(Moved to devel list)

> Somebody jumped on IRC the other day, and mentioned a missing
> functionality from the dev branch.  In stable, when hitting the
> "Addresses" button from compose, there are a series of "All" links
> above the check boxes.  The idea is to toggle the check boxes.

I'm not so sure we should call it "missing".  We might call it a "bug
in STABLE" instead.  :-)  That is, the "All" links encourage
abuse/mis-use of the To, Cc, Bcc fields -- in other words, lets users
use the address book like a mailing list.  Bad idea IMO.

> I went to duplicate this functionality onto dev in
> default/addrbook_search_list.tpl.  Code change is relatively simple,
> and keeping with the use_js idea, I put the JS inside the
> util_addressbook.php file, but when I went to test, the code didn't
> work.
>
> I found 2 different "bugs".  The first is the explicit hard coding of
> this variable, instead of basing values from the preferences, see:
>
> src/addrbook_search_html.php line 85
> src/addrbook_search_html.php line 117
> src/addrbook_search.php line 41
> src/addrbook_search.php line 86
>
> It appears this variable is only used in the adressbooks, and is set
> based on... who knows what.

I agree that this is a weird one-off variable that I've never seen
before either.  BUT, it's hard coded for the reason that the user
supposedly would not be able to use src/addrbook_search.php unless
JavaScript was working in the first place -- it's a script that is
ONLY used to populate a popup window.  The
src/addrbook_search_html.php file is the in-page version of the same
thing for people who don't have JavaScript working.

So, to me, it makes sense that you'd simply hard-code that value.
BUT, the thing is that the selection between the two modes is NOT
based on whether or not JavaScript is enabled -- it's a user
preference setting and nothing more.

So the solution would appear to br simply replace use_js with the
normal javascript_on (which should be available in ALL template files,
since it's assigned to the template in include/init.php).  The catch
here, though, is that the whole way src/addrbook_search.php works is
by using JavaScript to push addresses back to the "opener" page.  So
the second part of the solution is to override the user preference to
use the popup address book search when JavaScript is turned off.  In
that case, the hard coding of use_js in src/addrbook_search.php is
acceptable.  Only the hard coded value in src/addrbook_search_html.php
should be changed.

> The problem is, when the use_js is set to on, the check boxes are
> replaced with links, which appear to be dependent on the compose in
> new window setting, but are still broken anyway.

This is really just a bad design.  It's not that the page can operate
with JavaScript on or off no matter what, instead the use_js setting
*has* to be matched with how the search page is loaded - in a popup or
in-screen.  It should be called "used_in_popup" or something more
indicative of its application.  The point you raise here actually
raises an issue with my assumptions and "solution" above, too -- from
what you say, it's not about turning javascript on and off in any
other parts of the page -- it's JUST a control for how the addresses
are populated back to the compose screen and that's it (am I right?).
If that's the case, the hard coding is actually CORRECT (although we
should still check that the use of src/addrbook_search.php is
overridden when the user has JavaScript disabled).

  So the solution in this case is that if you are adding some little
javascript that does something else unrelated (see above, I'm not sure
we want to have a "Check All" link myself), you have to ignore use_js
(which is simply mis-named) and use "javascript_on" to do what you
need.

> I propose dropping the crazy links option when the use_js is on.  I
> also propose the use_js is dependent on the configuration/JS
> detection, and not hard coded to some value.

This isn't the right solution.

> The use_js option can then toggle the check boxes for if the "All"
> option should be available, as well as the code behind it.  The email
> addresses shouldn't be clickable when JS is off, because it uses JS
> functions that wouldn't be available (plus don't work when compose in
> new window is off, and when it's on, closes compose window).
>
> Any objections?

--
Paul Lesniewski
SquirrelMail Team
Please support Open Source Software by donating to SquirrelMail!
http://squirrelmail.org/donate_paul_lesniewski.php

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables
unlimited royalty-free distribution of the report engine
for externally facing server and web deployment.
http://p.sf.net/sfu/businessobjects
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Re: [SM-USERS] Curious use of "use_js_ in addresbook templates

by Jon Angliss :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, 16 May 2009 11:59:46 -0700, Paul Lesniewski
<paul@...> wrote:

>(Moved to devel list)

Thanks, wrote a better one shortly after I bodged this one :)

>> Somebody jumped on IRC the other day, and mentioned a missing
>> functionality from the dev branch.  In stable, when hitting the
>> "Addresses" button from compose, there are a series of "All" links
>> above the check boxes.  The idea is to toggle the check boxes.
>
>I'm not so sure we should call it "missing".  We might call it a "bug
>in STABLE" instead.  :-)  That is, the "All" links encourage
>abuse/mis-use of the To, Cc, Bcc fields -- in other words, lets users
>use the address book like a mailing list.  Bad idea IMO.

Not entirely sure I agree.  If I wanted to notify my famiy I'm going
to be away for a week, I might hit the "All" for the To box.  Not
really treating it as a mailing list.

>> I went to duplicate this functionality onto dev in
>> default/addrbook_search_list.tpl.  Code change is relatively simple,
>> and keeping with the use_js idea, I put the JS inside the
>> util_addressbook.php file, but when I went to test, the code didn't
>> work.
>>
>> I found 2 different "bugs".  The first is the explicit hard coding of
>> this variable, instead of basing values from the preferences, see:
>>
>> src/addrbook_search_html.php line 85
>> src/addrbook_search_html.php line 117
>> src/addrbook_search.php line 41
>> src/addrbook_search.php line 86
>>
>> It appears this variable is only used in the adressbooks, and is set
>> based on... who knows what.
>
>I agree that this is a weird one-off variable that I've never seen
>before either.  BUT, it's hard coded for the reason that the user
>supposedly would not be able to use src/addrbook_search.php unless
>JavaScript was working in the first place -- it's a script that is
>ONLY used to populate a popup window.  The
>src/addrbook_search_html.php file is the in-page version of the same
>thing for people who don't have JavaScript working.
>
>So, to me, it makes sense that you'd simply hard-code that value.
>BUT, the thing is that the selection between the two modes is NOT
>based on whether or not JavaScript is enabled -- it's a user
>preference setting and nothing more.
>
>So the solution would appear to br simply replace use_js with the
>normal javascript_on (which should be available in ALL template files,
>since it's assigned to the template in include/init.php).  The catch
>here, though, is that the whole way src/addrbook_search.php works is
>by using JavaScript to push addresses back to the "opener" page.  So
>the second part of the solution is to override the user preference to
>use the popup address book search when JavaScript is turned off.  In
>that case, the hard coding of use_js in src/addrbook_search.php is
>acceptable.  Only the hard coded value in src/addrbook_search_html.php
>should be changed.

The code is triggered from src/compose.php, there are traps around the
code to stop the wrong page from being loaded if JS is not available,
however the code that appears in the template still calls javascript
whether the option is enabled or not.  See line 84 of the template.
That javascript function does not exist when use_js is off.  This
breaks the email address being clickable, not that would work from the
in-page addressbook anyway.

>> The problem is, when the use_js is set to on, the check boxes are
>> replaced with links, which appear to be dependent on the compose in
>> new window setting, but are still broken anyway.
>
>This is really just a bad design.  It's not that the page can operate
>with JavaScript on or off no matter what, instead the use_js setting
>*has* to be matched with how the search page is loaded - in a popup or
>in-screen.  It should be called "used_in_popup" or something more
>indicative of its application.  The point you raise here actually
>raises an issue with my assumptions and "solution" above, too -- from
>what you say, it's not about turning javascript on and off in any
>other parts of the page -- it's JUST a control for how the addresses
>are populated back to the compose screen and that's it (am I right?).

I believe that's correct yes.

>If that's the case, the hard coding is actually CORRECT (although we
>should still check that the use of src/addrbook_search.php is
>overridden when the user has JavaScript disabled).

Right, I believe the search page for addresses is only ever triggered
from the compose page, and that's wrapped up nicely around line 1405
in src/compose.php.

>  So the solution in this case is that if you are adding some little
>javascript that does something else unrelated (see above, I'm not sure
>we want to have a "Check All" link myself), you have to ignore use_js
>(which is simply mis-named) and use "javascript_on" to do what you
>need.
>
>> I propose dropping the crazy links option when the use_js is on.  I
>> also propose the use_js is dependent on the configuration/JS
>> detection, and not hard coded to some value.
>
>This isn't the right solution.

Right, I sent the message prematurely, and was bashing my head right
after sending it when I was trying to figure out how the code was
triggered as it was.  See my follow up :)

--
Jonathan Angliss
<jon@...>


------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables
unlimited royalty-free distribution of the report engine
for externally facing server and web deployment.
http://p.sf.net/sfu/businessobjects
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Re: [SM-USERS] Curious use of "use_js_ in addresbook templates

by Paul Lesniewski :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

>>> Somebody jumped on IRC the other day, and mentioned a missing
>>> functionality from the dev branch.  In stable, when hitting the
>>> "Addresses" button from compose, there are a series of "All" links
>>> above the check boxes.  The idea is to toggle the check boxes.
>>
>>I'm not so sure we should call it "missing".  We might call it a "bug
>>in STABLE" instead.  :-)  That is, the "All" links encourage
>>abuse/mis-use of the To, Cc, Bcc fields -- in other words, lets users
>>use the address book like a mailing list.  Bad idea IMO.
>
> Not entirely sure I agree.  If I wanted to notify my famiy I'm going
> to be away for a week, I might hit the "All" for the To box.  Not
> really treating it as a mailing list.

Most people who use the address book seriously will have more than
their family in their address book (or any one category).  There won't
be many times that you'll want to send an email to every single person
in there, but anyway, I don't care that much about this (when my
address grouping plugin is integrated into this page, the "All" link
will probably be very appropriate anyway).

>>> I went to duplicate this functionality onto dev in
>>> default/addrbook_search_list.tpl.  Code change is relatively simple,
>>> and keeping with the use_js idea, I put the JS inside the
>>> util_addressbook.php file, but when I went to test, the code didn't
>>> work.
>>>
>>> I found 2 different "bugs".  The first is the explicit hard coding of
>>> this variable, instead of basing values from the preferences, see:
>>>
>>> src/addrbook_search_html.php line 85
>>> src/addrbook_search_html.php line 117
>>> src/addrbook_search.php line 41
>>> src/addrbook_search.php line 86
>>>
>>> It appears this variable is only used in the adressbooks, and is set
>>> based on... who knows what.
>>
>>I agree that this is a weird one-off variable that I've never seen
>>before either.  BUT, it's hard coded for the reason that the user
>>supposedly would not be able to use src/addrbook_search.php unless
>>JavaScript was working in the first place -- it's a script that is
>>ONLY used to populate a popup window.  The
>>src/addrbook_search_html.php file is the in-page version of the same
>>thing for people who don't have JavaScript working.
>>
>>So, to me, it makes sense that you'd simply hard-code that value.
>>BUT, the thing is that the selection between the two modes is NOT
>>based on whether or not JavaScript is enabled -- it's a user
>>preference setting and nothing more.
>>
>>So the solution would appear to br simply replace use_js with the
>>normal javascript_on (which should be available in ALL template files,
>>since it's assigned to the template in include/init.php).  The catch
>>here, though, is that the whole way src/addrbook_search.php works is
>>by using JavaScript to push addresses back to the "opener" page.  So
>>the second part of the solution is to override the user preference to
>>use the popup address book search when JavaScript is turned off.  In
>>that case, the hard coding of use_js in src/addrbook_search.php is
>>acceptable.  Only the hard coded value in src/addrbook_search_html.php
>>should be changed.
>
> The code is triggered from src/compose.php, there are traps around the
> code to stop the wrong page from being loaded if JS is not available,
> however the code that appears in the template still calls javascript
> whether the option is enabled or not.  See line 84 of the template.
> That javascript function does not exist when use_js is off.  This
> breaks the email address being clickable, not that would work from the
> in-page addressbook anyway.

That looks to be a mis-design.  Those links obviously were not given
any thought when not used in a popup.  To be used in-screen, each link
would need to contain all the hidden inputs needed to take the user
back to their compose session, which may or may not be realistic.  At
least short of being able to answer that question, the links should be
removed entirely when $use_js is off.

>>> The problem is, when the use_js is set to on, the check boxes are
>>> replaced with links, which appear to be dependent on the compose in
>>> new window setting, but are still broken anyway.
>>
>>This is really just a bad design.  It's not that the page can operate
>>with JavaScript on or off no matter what, instead the use_js setting
>>*has* to be matched with how the search page is loaded - in a popup or
>>in-screen.  It should be called "used_in_popup" or something more
>>indicative of its application.  The point you raise here actually
>>raises an issue with my assumptions and "solution" above, too -- from
>>what you say, it's not about turning javascript on and off in any
>>other parts of the page -- it's JUST a control for how the addresses
>>are populated back to the compose screen and that's it (am I right?).
>
> I believe that's correct yes.
>
>>If that's the case, the hard coding is actually CORRECT (although we
>>should still check that the use of src/addrbook_search.php is
>>overridden when the user has JavaScript disabled).
>
> Right, I believe the search page for addresses is only ever triggered
> from the compose page, and that's wrapped up nicely around line 1405
> in src/compose.php.
>
>>  So the solution in this case is that if you are adding some little
>>javascript that does something else unrelated (see above, I'm not sure
>>we want to have a "Check All" link myself), you have to ignore use_js
>>(which is simply mis-named) and use "javascript_on" to do what you
>>need.
>>
>>> I propose dropping the crazy links option when the use_js is on.  I
>>> also propose the use_js is dependent on the configuration/JS
>>> detection, and not hard coded to some value.
>>
>>This isn't the right solution.
>
> Right, I sent the message prematurely, and was bashing my head right
> after sending it when I was trying to figure out how the code was
> triggered as it was.  See my follow up :)

Hmm, all I saw was something to the effect of "please ignore this".....?

--
Paul Lesniewski
SquirrelMail Team
Please support Open Source Software by donating to SquirrelMail!
http://squirrelmail.org/donate_paul_lesniewski.php

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables
unlimited royalty-free distribution of the report engine
for externally facing server and web deployment.
http://p.sf.net/sfu/businessobjects
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Re: [SM-USERS] Curious use of "use_js_ in addresbook templates

by Jon Angliss :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Sat, 16 May 2009 14:53:28 -0700, Paul Lesniewski
<paul@...> wrote:

>>>> Somebody jumped on IRC the other day, and mentioned a missing
>>>> functionality from the dev branch.  In stable, when hitting the
>>>> "Addresses" button from compose, there are a series of "All" links
>>>> above the check boxes.  The idea is to toggle the check boxes.
>>>
>>>I'm not so sure we should call it "missing".  We might call it a "bug
>>>in STABLE" instead.  :-)  That is, the "All" links encourage
>>>abuse/mis-use of the To, Cc, Bcc fields -- in other words, lets users
>>>use the address book like a mailing list.  Bad idea IMO.
>>
>> Not entirely sure I agree.  If I wanted to notify my famiy I'm going
>> to be away for a week, I might hit the "All" for the To box.  Not
>> really treating it as a mailing list.
>
>Most people who use the address book seriously will have more than
>their family in their address book (or any one category).  There won't
>be many times that you'll want to send an email to every single person
>in there, but anyway, I don't care that much about this (when my
>address grouping plugin is integrated into this page, the "All" link
>will probably be very appropriate anyway).

Those that use it extensively yes, will more than likely have more
than a few entries.  Just think it's one of those features that
probably more people use that we think about.

>>>> I went to duplicate this functionality onto dev in
>>>> default/addrbook_search_list.tpl.  Code change is relatively simple,
>>>> and keeping with the use_js idea, I put the JS inside the
>>>> util_addressbook.php file, but when I went to test, the code didn't
>>>> work.
>>>>
>>>> I found 2 different "bugs".  The first is the explicit hard coding of
>>>> this variable, instead of basing values from the preferences, see:
>>>>
>>>> src/addrbook_search_html.php line 85
>>>> src/addrbook_search_html.php line 117
>>>> src/addrbook_search.php line 41
>>>> src/addrbook_search.php line 86
>>>>
>>>> It appears this variable is only used in the adressbooks, and is set
>>>> based on... who knows what.
>>>
>>>I agree that this is a weird one-off variable that I've never seen
>>>before either.  BUT, it's hard coded for the reason that the user
>>>supposedly would not be able to use src/addrbook_search.php unless
>>>JavaScript was working in the first place -- it's a script that is
>>>ONLY used to populate a popup window.  The
>>>src/addrbook_search_html.php file is the in-page version of the same
>>>thing for people who don't have JavaScript working.
>>>
>>>So, to me, it makes sense that you'd simply hard-code that value.
>>>BUT, the thing is that the selection between the two modes is NOT
>>>based on whether or not JavaScript is enabled -- it's a user
>>>preference setting and nothing more.
>>>
>>>So the solution would appear to br simply replace use_js with the
>>>normal javascript_on (which should be available in ALL template files,
>>>since it's assigned to the template in include/init.php).  The catch
>>>here, though, is that the whole way src/addrbook_search.php works is
>>>by using JavaScript to push addresses back to the "opener" page.  So
>>>the second part of the solution is to override the user preference to
>>>use the popup address book search when JavaScript is turned off.  In
>>>that case, the hard coding of use_js in src/addrbook_search.php is
>>>acceptable.  Only the hard coded value in src/addrbook_search_html.php
>>>should be changed.
>>
>> The code is triggered from src/compose.php, there are traps around the
>> code to stop the wrong page from being loaded if JS is not available,
>> however the code that appears in the template still calls javascript
>> whether the option is enabled or not.  See line 84 of the template.
>> That javascript function does not exist when use_js is off.  This
>> breaks the email address being clickable, not that would work from the
>> in-page addressbook anyway.
>
>That looks to be a mis-design.  Those links obviously were not given
>any thought when not used in a popup.  To be used in-screen, each link
>would need to contain all the hidden inputs needed to take the user
>back to their compose session, which may or may not be realistic.  At
>least short of being able to answer that question, the links should be
>removed entirely when $use_js is off.

Right, that was what I was thinking.  I'll fix it up.

>>>> The problem is, when the use_js is set to on, the check boxes are
>>>> replaced with links, which appear to be dependent on the compose in
>>>> new window setting, but are still broken anyway.
>>>
>>>This is really just a bad design.  It's not that the page can operate
>>>with JavaScript on or off no matter what, instead the use_js setting
>>>*has* to be matched with how the search page is loaded - in a popup or
>>>in-screen.  It should be called "used_in_popup" or something more
>>>indicative of its application.  The point you raise here actually
>>>raises an issue with my assumptions and "solution" above, too -- from
>>>what you say, it's not about turning javascript on and off in any
>>>other parts of the page -- it's JUST a control for how the addresses
>>>are populated back to the compose screen and that's it (am I right?).
>>
>> I believe that's correct yes.
>>
>>>If that's the case, the hard coding is actually CORRECT (although we
>>>should still check that the use of src/addrbook_search.php is
>>>overridden when the user has JavaScript disabled).
>>
>> Right, I believe the search page for addresses is only ever triggered
>> from the compose page, and that's wrapped up nicely around line 1405
>> in src/compose.php.
>>
>>>  So the solution in this case is that if you are adding some little
>>>javascript that does something else unrelated (see above, I'm not sure
>>>we want to have a "Check All" link myself), you have to ignore use_js
>>>(which is simply mis-named) and use "javascript_on" to do what you
>>>need.
>>>
>>>> I propose dropping the crazy links option when the use_js is on.  I
>>>> also propose the use_js is dependent on the configuration/JS
>>>> detection, and not hard coded to some value.
>>>
>>>This isn't the right solution.
>>
>> Right, I sent the message prematurely, and was bashing my head right
>> after sending it when I was trying to figure out how the code was
>> triggered as it was.  See my follow up :)
>
>Hmm, all I saw was something to the effect of "please ignore this".....?

Should have hit the dev list... Guessing it's taking the long route
because I'm posting through gmane :)  It basically summized the same
thing you did, but proposed a slightly different solution, a second
template.  I'll tinker with the template and get it working with just
the one, and changing use_js to be more appropriately named, along
with adding another for really using JS :)

--
Jonathan Angliss
<jon@...>


------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables
unlimited royalty-free distribution of the report engine
for externally facing server and web deployment.
http://p.sf.net/sfu/businessobjects
-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: squirrelmail-devel@...
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel