+++ This bug was initially created as a clone of Bug #406181 +++
Description:
On certain pages Ajax optimizations are used to speed up page loading time. Red
Hat has a high number of components for certain products. The component lists
cause a large amount of HTML having to be downloaded by the client browser. The
Ajax functions allow the UI to only show the components needed depending on
which products are selected.
Function Requirements:
Javascript code has been added to specific cgi's to detect whether javascript is
enabled for the clients browser. We then use this information to decide whether
to display the ajax optimizations or to display the legacy page.

Created attachment 299968[details]
Patch to add javascript enabled check to Bugzilla code (v1)
I have created a patch to determine whether a user's browser supports
javascript. It adds a new column to the profiles table called javascript that
is either 1 or 0 based on whether the Bugzilla_javascript cookie is found. The
cookie check happens in Bugzilla/Auth/Verify/Cookie.pm where it will update the
javascript value in profiles for the user who is verified. The
Bugzilla_javascript cookie is then deleted for verification again next time. In
template/en/default/global/header.html.tmpl setcookie('Bugzilla_javascript', 1)
is called in the <body onload> tag. This part is similar to how we do it in
2.18. The difference is we are now storing it in the profiles table so that it
becomes part of the normal User.pm object and can be used anywhere that
Bugzilla->user is available. If the browser does not support javascript,
setcookie() fails to work and profiles.javascript is always 0.
Please review
Dave

Comment on attachment 299968[details]
Patch to add javascript enabled check to Bugzilla code (v1)
Scratch this one, only works for logged in users which is not fully what we
need. This needs to also work with users who are not logged in so that they can
still take advantage of the query.cgi enhancements.
Working on new patch.

Created attachment 300139[details]
Patch to add javascript enabled check to Bugzilla code (v2)
New patch. Set up User.pm now to properly have setters and accessors instead of
hitting the DB directly in Bugzilla::Auth::Login::Cookie::get_login_info().
Also the javascript flag should now be set properly even for users who are not
logged in. Also fixed a bug with setcookie() where I wasn't specifying exact
domain and path info so the cookie couldn't be found properly when it came time
to clear it.
Please review.
Dave

Comment on attachment 300139[details]
Patch to add javascript enabled check to Bugzilla code (v2)
This is one bug I would really like to see done in the upstream. There are 100
different ways it could be implemented and it would benefit from the attention
of many eyeballs.
Regarding the patch, it looks sane, the only issue I have with it is that we
are customising again. But you've all heard that before :D Still, we have
deadlines so I guess the best we can do is create a bug in the upstream and
hope that it gets pulled into the head for ver4.

(In reply to comment #6)
> (From update of attachment 300139[details] [edit])
> This is one bug I would really like to see done in the upstream. There are 100
> different ways it could be implemented and it would benefit from the attention
> of many eyeballs.
>
I agree. I can see the comments already. "Why do you have so many components
anyway?", "You should code the page in such a way that it doesn't need the
detection", "Why is Red Hat doing so much crack? ;)", etc.
> Regarding the patch, it looks sane, the only issue I have with it is that we
> are customising again. But you've all heard that before :D Still, we have
> deadlines so I guess the best we can do is create a bug in the upstream and
> hope that it gets pulled into the head for ver4.
Will do and see what happens. One of the reasons I decided to store the value in
the database which is different that what we do in 2.18 is due to "RT #18340:
Bugzilla - record which users have javascript cookie set". We can then look back
at the profiles table and get an idea on how many users visit our site with
javascript enabled. Yet to be seen how useful the stats are, I can already
assume that most people are connecting with either Firefox/Safari/IE and the
majority of them will have javascript enabled. It might be useful to also store
the UserAgent as well in the profiles table.
As for the patch, I wonder if this would be best stored in the logincookies
table instead of profiles. Then we could track javascript usage per session.
With it in the profiles table it just gets overwritten each time.
What do you think
Dave

> majority of them will have javascript enabled. It might be useful to also
store
> the UserAgent as well in the profiles table.
That info should already be in the apache logs. So do we really want to store
it in the profiles table?
> As for the patch, I wonder if this would be best stored in the logincookies
> table instead of profiles. Then we could track javascript usage per session.
> With it in the profiles table it just gets overwritten each time.
>
> What do you think
Depends how you want to model it. En/Disabling javascript per session points
to the logincookies table. The profiles.javascript field could be indicate
users general preference towards javascript.
Could be too complicated and most people's heads will explode (WTF? I thought
I enabled javascript)?

(In reply to comment #9)
> (From update of attachment 300139[details] [edit])
> Hi Dave, patch looks good to me too. on 2 notes below.
>
> >Index: Bugzilla/Auth.pm
> >===================================================================
> >+ # REDHAT EXTENSION START 426386
> >+ # Set javascript flag to enable UI enhancements
> >+ $user->set_javascript($result->{javascript}) if exists
$result->{javascript};
> > }
> >
>
> shall we here update the database by $user->update so it will be:
>
> if (exists $result->{javascript}){
> $user->set_javascript($result->{javascript});
> $user->update;
> }
I dont use $user->update here since this section is only for users who are not
logged in and get the default user object which has $user->id == 0. This $user
object only lives for the live of the current page. $user->update is called
however after I do $user->set_javascript($result->{javascript}) in
Bugzilla/Auth/Verify.pm line 129 which is for when the user is logged in.
>
> >Index: Bugzilla/User.pm
> >===================================================================
> >
> >+# REDHAT EXTENSION START 426386
> >+sub set_javascript { $_[0]->set('javascript', $_[1]) }
>
> just missing a semicolon at the end of the statement
Thanks

Dave,
what about your idea of tracking the javascript in the logincookies table?
profiles.javascript - indicates a general user preference
logincookies.javascript - indicates the preference for the session.
Time constraints may make this decision for you. I figure storing it in the
profiles table will work for 99% of the use cases. The logincookies would
satisfy that rare animal that wants to run two or more sessions
simultaneously, some with js off, some with js on. Surely a rare usecase? And
probably not worth the effort of a re-write now.
I think I just convinced myself that the wise course is to go with this patch
for now. Unless anyone complains majorly we put off work on the new improved
version until it can be done with the upstream.

Another thought.
Is it even worth updating the profiles table with the javascript value? On the
plus side we record a history of who is using javascript, valuable info. On
the minus we have to update the profiles table on each click for logged in
users. Another pointer that it belongs on the logincookies table as we update
that anyhow, right?
Just thinking out aloud.

(In reply to comment #11)
> Dave,
>
> what about your idea of tracking the javascript in the logincookies table?
>
> profiles.javascript - indicates a general user preference
> logincookies.javascript - indicates the preference for the session.
>
> Time constraints may make this decision for you. I figure storing it in the
> profiles table will work for 99% of the use cases. The logincookies would
> satisfy that rare animal that wants to run two or more sessions
> simultaneously, some with js off, some with js on. Surely a rare usecase? And
> probably not worth the effort of a re-write now.
XMLRPC clients.
>
> I think I just convinced myself that the wise course is to go with this patch
> for now. Unless anyone complains majorly we put off work on the new improved
> version until it can be done with the upstream.
>
Let me quickly look and see what type of work it would take to do this. It
may be better to put in the logincookies table. We have to update the
logincookies.lastused date column anyway.
Dave

Created attachment 302670[details]
Patch to add javascript enabled check to Bugzilla code - logincookies (v1)
Attaching a patch that uses logincookies.javascript instead of the profiles
table. It still updates the User.pm object with the value for user.javascript
but no longer stores it in the profiles table.
Please review
Dave

Comment on attachment 302676[details]
Patch to add javascript enabled check to Bugzilla code - logincookies (v1)
So the difference is that instead of adding a db field to profiles we add it to
logincookies. BUT the javascript boolean is still available by querying the
javascript method on the User object. Correct? If so, should set_javascript
still be calling set?

Dave I tried testing this on bugdev. Although i logged in and i have javascript
enabled ,, I checked the bz-db1-test bugs database logincookies table and
couldn't see that javascript field was set to 1 for my sessions, it was set to
0. am i misunderstanding anything?
Noura

(In reply to comment #18)
> (From update of attachment 302676[details] [edit])
> So the difference is that instead of adding a db field to profiles we add it to
> logincookies. BUT the javascript boolean is still available by querying the
> javascript method on the User object. Correct? If so, should set_javascript
> still be calling set?
>
I removed the profiles.javascript column from the UPDATE_COLUMNS sub so even
though I am called set_javascript() which calls set() it will not try to update
the value in the profiles table. It will however update the value in the current
User.pm instance so that for all of the code it looks like another attribute of
the user.
Dave

(In reply to comment #19)
> Dave I tried testing this on bugdev. Although i logged in and i have javascript
> enabled ,, I checked the bz-db1-test bugs database logincookies table and
> couldn't see that javascript field was set to 1 for my sessions, it was set to
> 0. am i misunderstanding anything?
>
> Noura
My fault. I did not have the patch installed on bugdev so that will explain why
it was not working. i am going to go ahead and check this into cvs since some of
the other patches depend on it and this is lower danger than the other patches.
Dave

Note

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