Last Comment Bug 584742 - In show_bug.cgi, WebKit based browsers can automatically reset a field's selected value, when the field has disabled values
: In show_bug.cgi, WebKit based browsers can automatically reset a field's sele...
Status: RESOLVED WORKSFORME
:
Product: Bugzilla
Classification: Server Software
Component: Creating/Changing Bugs (show other bugs)
: 4.1
: All All
: -- major with 2 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
: default-qa
Mentors:
: 630907 641450 666747 (view as bug list)
Depends on: 636416
Blocks:
  Show dependency treegraph
 
Reported: 2010-08-05 09:23 PDT by Kent Rogers [:mrbball]
Modified: 2014-03-04 09:09 PST (History)
10 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Example without YUI (2.15 KB, text/html)
2010-08-06 10:16 PDT, Guy Pyrzak
no flags Details
Hacky Fix (355 bytes, patch)
2010-10-27 15:09 PDT, Guy Pyrzak
no flags Details | Diff | Review

Description Kent Rogers [:mrbball] 2010-08-05 09:23:03 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Bugzilla 4.1

When there are some disabled values for particular a Bugzilla field, the
combination of javascript in show_bug.cgi and the html that is generated
for the disabled fields seems to combine to allow the field's value to be
automatically re-set when navigating back to the bug from another page
(e.g. the bug's Activity Log).  This only seems to happen on WebKit based
browsers.

This is a fairly serious problem, IMO.  It has affected many bugs in our
installation, which now have incorrect values for the Platform field.
It has caused a fair amount of confusion and lost productivity as we
find/reset these incorrect values.

Reproducible: Always

Steps to Reproduce:
1. Open a WebKit based browser such as Safari
2. Go to a bug on a Bugzilla installation where some values for the
   Platform field are "disabled"
3. Navigate to a particular bug
4. Click the "History" link
5. Click the back button of the browser

Upon return to the bug display, the selected value for the Platform
field may have automatically been reset!

I was able to reproduce this specifically by setting up a new Bugzilla
installation from tip (on 8/04).  I created a few bugs, all with a
Platform of "Other", then disabled the Platform value "PC".  After
navigating to one of the bugs, I clicked the "History" link and then
clicked the back button of the browser.  The value for the Platform
field was automatically reset to "Macintosh".

I have seen this when using Safari, but NOT when using Firefox.
Someone else here has seen it on other WebKit based browsers as well:
Google Chrome 5.0.375.125 (Windows 7) and iOS 4.0.1.


Expected Results:  
Field values should remain what they were prior to navigating away from
show_bug.cgi.

I believe I have narrowed down the specific combination of javascript &
html that causes this to the following.  This html displays a single
select button which automatically changes value when you navigate away
and then navigate back using the browser's back button:

<html>
  <head>
    <title>Bug 1 &ndash; testing</title>

<script type="text/javascript" src="js/yui/yahoo-dom-event/yahoo-dom-event.js?1277122537"></script><script type="text/javascript" src="js/yui/cookie/cookie-min.js?1277122537"></script><script type="text/javascript" src="js/yui/datasource/datasource-min.js?1277122537"></script><script type="text/javascript" src="js/yui/connection/connection-min.js?1277122537"></script><script type="text/javascript" src="js/yui/json/json-min.js?1277122537"></script><script type="text/javascript" src="js/yui/autocomplete/autocomplete-min.js?1277122537"></script><script type="text/javascript" src="js/yui/calendar/calendar-min.js?1277122537"></script><script type="text/javascript" src="js/global.js?1266418978"></script>

<script type="text/javascript" src="js/util.js?1271184061"></script><script type="text/javascript" src="js/field.js?1280948248"></script>
  </head>

  <body>
      <select id="rep_platform" 
                name="rep_platform" 
                >
            <option value="All"
                    id="v1_rep_platform"
              >All</option>
            <option value="PC"
                    id="v2_rep_platform"
              
                class="bz_hidden_option" disabled="disabled">PC</option>
            <option value="Macintosh"
                    id="v3_rep_platform"
              >Macintosh</option>
            <option value="Other"
                    id="v4_rep_platform"
              
                selected="selected">Other</option>
        </select>
        

        <script type="text/javascript">
        <!--
          initHidingOptionsForIE('rep_platform');
          
        //-->
        </script>

</body>
</html>
Comment 1 Frédéric Buclin 2010-08-05 11:06:15 PDT
Are the YUI scripts really needed in your testcase to reproduce your issue? If yes, maybe it is related to bug 577574.
Comment 2 Kent Rogers [:mrbball] 2010-08-05 16:41:43 PDT
(In reply to comment #1)
> Are the YUI scripts really needed in your testcase to reproduce your issue? If
> yes, maybe it is related to bug 577574.

I guess not.  It's difficult to test since I don't have regular access to Safari, but I
was still able to reproduce the problem after eliminating all of the YUI scripts
except the first one:

<script type="text/javascript" src="js/yui/yahoo-dom-event/yahoo-dom-event.js?1277122537"></script>
Comment 3 Max Kanat-Alexander 2010-08-05 16:52:48 PDT
My suspicion is that this is indeed related to the unload listener, but I'm not sure. It doesn't happen in earlier versions of Bugzilla? (The ability to disable field values has been around for longer than 4.1.)
Comment 4 Kent Rogers [:mrbball] 2010-08-05 17:01:29 PDT
It does happen in earlier versions.  We run a modified 3.4.2 in production and it
happens there, but tip is where I verified it outside of my local customizations.
Comment 5 Guy Pyrzak 2010-08-06 08:36:48 PDT
commenting out initHidingOptionsForIE also fixes the problem...

Do we need this for safari browsers the name would indicate we didn't
Comment 6 Guy Pyrzak 2010-08-06 09:16:12 PDT
yup. adding 

YAHOO.util.Event._simpleRemove(window, "unload", YAHOO.util.Event._unload);

to the example in the attachment fixes the bug. We should tell the YUI folks about this.

In the mean time i am investigating using the pagehide event instead of unload, maybe it will fix things in safari and firefox?
Comment 7 Guy Pyrzak 2010-08-06 09:22:54 PDT
Ok, http://webkit.org/blog/516/webkit-page-cache-ii-the-unload-event/

indicates that pagehide will fix this bug and also fix our autocomplete bug. So I'm thinking we change the if gecko to be if gecko or webkit and change the unload listener to be a pagehide. We should also talk to the YUI guys about this. It seems both webkit and firefox would prefer pagehide to unload.
Comment 8 Guy Pyrzak 2010-08-06 10:16:03 PDT
Created attachment 463585 [details]
Example without YUI

So here is an example of the same bug. However, without YUI, it is still reproducible in chrome, but not in safari.

I'm happy to remove the unload event and replace it with a page hide for all browsers that support it but I don't think this will fix the problem with chrome.
Comment 9 Max Kanat-Alexander 2010-08-06 17:59:42 PDT
(In reply to comment #5)
> Do we need this for safari browsers the name would indicate we didn't

  We do. We need it for any browser that doesn't understand "display: none" on <option> tags.
Comment 10 Max Kanat-Alexander 2010-08-06 18:01:46 PDT
  It might also be possible to fix initHidingOptionsForIE so that it uses the currently-selected default instead of the one in the template. Not sure, though.
Comment 11 Guy Pyrzak 2010-08-17 08:32:55 PDT
nm, this bug happens in safari and chrome with the attachment i gave
Comment 12 Kent Rogers [:mrbball] 2010-10-27 10:32:24 PDT
(In reply to comment #6)
> yup. adding 
> 
> YAHOO.util.Event._simpleRemove(window, "unload", YAHOO.util.Event._unload);
> 
> to the example in the attachment fixes the bug. We should tell the YUI folks
> about this.
> 
> In the mean time i am investigating using the pagehide event instead of unload,
> maybe it will fix things in safari and firefox?
>
Is there a workaround here that I can add to my code base?

This problem is just killing us and I'll do whatever I can to work around
it until a permanent fix is deployed.
Comment 13 Guy Pyrzak 2010-10-27 11:32:29 PDT
(In reply to comment #12)
> This problem is just killing us and I'll do whatever I can to work around
> it until a permanent fix is deployed.

I THINK this was fixed in another bug. Check out landfill's "bleeding edge" demo

if that's the case here is the code that fixed it:

        if ( "onpagehide" in window || YAHOO.env.ua.gecko) {
            YAHOO.util.Event._simpleRemove(window, "unload", 
                                           YAHOO.util.Event._unload);
            YAHOO.util.Event._simpleAdd(window, "pagehide", 
                                           YAHOO.util.Event._unload);
        }

Just put that in the global header template template/en/default/global/header.html.tmpl
Comment 14 Kent Rogers [:mrbball] 2010-10-27 11:47:44 PDT
That doesn't solve the problem for me (using Chrome).
Comment 15 Max Kanat-Alexander 2010-10-27 13:07:02 PDT
  Chrome has no page cache, so the state of JS is not saved. There may be something we can do with initOptionsForIE to make things work better if something is already selected, though.
Comment 16 Guy Pyrzak 2010-10-27 15:09:51 PDT
Created attachment 486461 [details] [diff] [review]
Hacky Fix

so the bug is basically caused by browserCanHideOptions. It incorrectly thinks that webkit can't hide stuff, which is wrong. 

So the super easy fix that max won't like is attached. It basically says ALL webkit browsers support can_hide_options
Comment 17 Kent Rogers [:mrbball] 2010-10-28 06:30:14 PDT
I see the code in comment 13 fixed the problem for Safari, and that will
be a big help.

But, even the "Hacky Fix" doesn't solve the problem for Chrome.
Comment 18 Guy Pyrzak 2010-10-28 10:39:41 PDT
the fix that I posted should work the same in all webkit browsers (safari and chrome) try shift refreshing. I just double checked and YAHOO.env.ua.webkit != 0 for chrome. The code that was causing the bug no longer being called, so I'm not sure how you're still running into the bug.
Comment 19 Kent Rogers [:mrbball] 2010-10-29 05:36:35 PDT
I shift/refreshed until I was blue in the face and it made no difference.
Only once I cleared the entire cache did it start working.  Not sure what's
going on there, but it works great now!
Comment 20 Frédéric Buclin 2011-02-02 10:33:22 PST
*** Bug 630907 has been marked as a duplicate of this bug. ***
Comment 21 Frédéric Buclin 2011-03-14 10:17:05 PDT
*** Bug 641450 has been marked as a duplicate of this bug. ***
Comment 22 Byron Jones ‹:glob› 2011-03-14 10:29:40 PDT
i've been investigating this; while the issue is caused by browserCanHideOptions, it appears that only IE has issues with disabled options. (see bug 641450).

i'm yet to test with an old chrome version, however the oldest version of safari i could get my hands on (v3.2) doesn't have any issues with disabled options, so it's reasonable to assume that old chrome versions also won't have options.

i propose browserCanHideOptions is changed to:

function browserCanHideOptions(aSelect) {
    // IE is the only modern borwser which has issues with disabled options
/*@cc_on @*/
/*@if (@_win32)
    return false;
/*@end @*/
    return true;
}

guy/mkanat: what do you think?
Comment 23 Max Kanat-Alexander 2011-03-14 15:26:05 PDT
(In reply to comment #22)
> guy/mkanat: what do you think?

  No, that would not work. browserCanHideOptions must return false for every browser except Gecko currently.

  I suspect that my recent refactoring of this code may actually fix this problem.
Comment 24 Byron Jones ‹:glob› 2011-03-14 21:36:04 PDT
>   No, that would not work. browserCanHideOptions must return false for every
> browser except Gecko currently.

actually it does work. i've tested it in safari, chrome, opera, firefox and ie.
Comment 25 Max Kanat-Alexander 2011-03-14 21:46:31 PDT
(In reply to comment #24)
> actually it does work. i've tested it in safari, chrome, opera, firefox and ie.

  Are you assuming that the code is supposed to only disable options? It is not, it is supposed to entirely remove them from visibility.
Comment 26 Byron Jones ‹:glob› 2011-04-06 14:13:17 PDT
what we're doing currently is the template is putting the <option> into the html, then we're using javascript to remove the <option>.

wouldn't it be much simpler to not output disabled <options> unless they are the currently selected?
Comment 27 Max Kanat-Alexander 2011-04-07 16:30:22 PDT
For those reading this bug, glob and I discussed his question in person. This code is primarily used for "value controllers" and has a lot of value there.
Comment 28 Max Kanat-Alexander 2011-07-01 15:20:13 PDT
*** Bug 666747 has been marked as a duplicate of this bug. ***
Comment 29 Frédéric Buclin 2011-08-12 04:59:56 PDT
mkanat, glob: is this bug fixed by bug 636416? The status whiteboard should be updated.
Comment 30 Byron Jones ‹:glob› 2011-08-12 05:13:23 PDT
(In reply to Frédéric Buclin from comment #29)
> mkanat, glob: is this bug fixed by bug 636416? The status whiteboard should
> be updated.

bug 636416 applied to search only; work needs to happen here to use the value-controller on show_bug.
Comment 31 Max Kanat-Alexander 2011-08-15 15:53:21 PDT
(In reply to Byron Jones ‹:glob› from comment #30)
> bug 636416 applied to search only; work needs to happen here to use the
> value-controller on show_bug.

  The value-controller was already what was being used on show_bug; that's actually where it comes from. :-) Is the code being used for disabled values though? That I do not know.
Comment 32 Byron Jones ‹:glob› 2011-08-15 19:36:01 PDT
(In reply to Max Kanat-Alexander from comment #31)
>   The value-controller was already what was being used on show_bug; that's
> actually where it comes from. :-) Is the code being used for disabled values
> though? That I do not know.

clearly not, given the existence of this bug and its many duplicates.
Comment 33 Max Kanat-Alexander 2011-08-16 17:31:49 PDT
(In reply to Byron Jones ‹:glob› from comment #32)
> clearly not, given the existence of this bug and its many duplicates.

  The updates to the value-controller code that would have fixed this just went in a few days ago, long before all of these bugs were filed.
Comment 34 Michael Shigorin 2012-03-01 13:00:47 PST
Seems I've seen something like this on 3.x (3.2/3.6, particularly) using Firefox 3.6+.
Comment 35 Joseph Carroll 2013-02-21 12:43:23 PST
As a guy who is not familiar with any of the code mentioned in this bug, what is the status of this bug as of today?

We are evaluating moving from Bugzilla 2.22.1 to 4.4.rc2. (any idea when there might be an official 4.4 release?)  I realize that we are a little behind.

Anyways, I feel as though this bug would be something that could delay any decision to move forward with a new release.

Any comments / suggestions appreciated.

Thanks,

JD
Comment 36 Frédéric Buclin 2013-02-21 14:44:58 PST
I'm unable to reproduce this issue with Chrome 24, neither on Bugzilla 3.6.13 nor 4.4rc2. Can someone still reproduce with a *recent* version of Chrome?
Comment 37 Frédéric Buclin 2014-03-04 03:46:13 PST
Kent: can you still reproduce this problem with Bugzilla 4.4?
Comment 38 Kent Rogers [:mrbball] 2014-03-04 05:46:09 PST
(In reply to Frédéric Buclin from comment #37)
> Kent: can you still reproduce this problem with Bugzilla 4.4?
>
I installed Guy's "Hacky Fix" years ago and have not had any problems in my
installation since.

But, I also cannot reproduce this using Chrome 22 on a stock 4.4.2+ installation.
Comment 39 Frédéric Buclin 2014-03-04 09:09:43 PST
OK, so closing this bug.

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