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

On Sat, 16 May 2009 14:53:28 -0700, Paul Lesniewski
<[email protected]> 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
<[email protected]>


------------------------------------------------------------------------------
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: [email protected]
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel