Closed Bug 584742 Opened 14 years ago Closed 10 years ago

In show_bug.cgi, WebKit based browsers can automatically reset a field's selected value, when the field has disabled values

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

Product:
Bugzilla
Component:
Creating/Changing Bugs
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: mrbball, Unassigned)

References

Details

Attachments

(2 files)

Example without YUI
2.15 KB, text/html
Details
Hacky Fix
355 bytes, patch
Details | Diff | Splinter Review 
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>
Are the YUI scripts really needed in your testcase to reproduce your issue? If yes, maybe it is related to bug 577574.
Version: unspecified → 4.1
(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>
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.)
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.
commenting out initHidingOptionsForIE also fixes the problem...

Do we need this for safari browsers the name would indicate we didn't
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?
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached file 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.
(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.
  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.
nm, this bug happens in safari and chrome with the attachment i gave
(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.
(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
OS: Mac OS X → Windows 7
That doesn't solve the problem for me (using Chrome).
OS: Windows 7 → Windows XP
  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.
Attached patch Hacky Fix — — Splinter Review 
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
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.
OS: Windows XP → All
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.
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!
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?
(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.
Depends on: 636416
Whiteboard: [blocker will fix?]
>   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.
(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.
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?
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.
mkanat, glob: is this bug fixed by bug 636416? The status whiteboard should be updated.
(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.
Whiteboard: [blocker will fix?]
(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.
(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.
(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.
Seems I've seen something like this on 3.x (3.2/3.6, particularly) using Firefox 3.6+.
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
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?
Kent: can you still reproduce this problem with Bugzilla 4.4?
Flags: needinfo?(kar)
(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.
Flags: needinfo?(kar)
OK, so closing this bug.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: