_form_builder_ie_cleanup not working -search submission

I'm not experiencing this issue in either Beta 3 or 6.x-dev. Mind testing it in 6.x-dev? What browser are you using?

The issue is Internet Explorer under Windows. It happens in IE6 and IE7 in both the Search block and the Search page. Damn browsers cause so many problems. Works in every other browser...

ssh: critical is reserved to far more important problems. Like you cannot edit your menu items.

ssb: this search form is not new and it probably had the same bug for a few releases. If not, and previous ones worked better, feel free to analyze and come up with what's different and what needs to be fixed. This bug will surely be fixed in Drupal if someone provides a working patch. Otherwise not.

http://drupal.org/node/199198 was duplicate.

According to http://www.upsdell.com/BrowserNews/stat_trends.htm (Jan 5, 2008):

Roughly 79% use IE-based browsers, down from a high of ~94% as users switched to other browser families — mainly Gecko and KHTML. • About 30-35% use IE7, with its percentage growing slowly. Slightly more use IE6, with its percentage decreasing slowly as users switch to IE7 and other browsers. • Many still use IE5, with the number shrinking slowly as users upgrade or switch; IE5 will remain a factor for several years, especially since IE7 cannot be installed on Windows older than XP SP2, and since many companies using Windows 2000, which came with IE5, are refusing to update to IE6.

I agree with ssb. While I do NOT like IE, it is a reality, and Drupal developers ignore it unwisely.

sunfish62: who ignores this? We said we are open to dicussing fixes and improvements. First is to check whether this is new in Drupal 6, as I noted above.

ssb: Although some of us probably have better PHP experience then you do (not that there is any PHP experience required here), we might not have an Internet Explorer install to test with. So there needs to be some cross between those who can reproduce the bug in Internet Explorer and those who can look into fixing it.

I have IE6 installed on my Mac OS X with IEs4OSX (http://hojtsy.hu/blog/2007-dec-24/ies4osx-saved-drupal-hu-christmas) and went ahead to test the search block form. When I clicked on the search input field in Drupal 5, the search button was automatically selected and the submission went fine. The same did not happen on Drupal 6. So reproduced.

Now let's see what happens in the HTML (note there is no elit PHP skills required here, just copying source from browser): http://drupalbin.com/464

Look, the difference is that the submit action is different. In Drupal 5, it is always the node form, in Drupal 6 it is always the same URL. Well. Now go check why it works when you click the button, not press enter. Since the HTML form is the same, the browser initiated action should be different. This is an issue somewhere on the browser side, possibly JavaScript.

Sorry, but I am not a JS guru, and a quick search for submit handlers in the Drupal source code did not turn up anything either. Feel free to take it on from here.

A related bug may be that the search block doesn't appear to work on the same page as the search form (via theme) - although iirc that's a drupal 5 issue as well. Will try to look into that a bit closer later on.

edit: well I didn't check 5, but it's no longer an issue on 6.x if it ever was.

I think I might have tracked this down.

IE7 apparently treats name and id exactly the same: http://remysharp.com/2007/02/10/ie-7-breaks-getelementbyid/

The html for the search form has name and id both set to search-theme-form on different elements.

Not to mention the duplicate id code seems to have added -1 to the textfield id as well, which probably doesn't have anything to do with this, but it shows these are a bit too closely named.

(note, no php knowledge required).

The search block form has exactly the same issue, except the id/names are search_block_form instead.

This is likely to affect any javascript dependent on ids as well. Moving to forms system.

*scratching primate like head* So.. is there a recommended solution to this or just use the forms api instead?

No solution yet no, I may not have even identified the bug correctly.

What you could do is copy the search-theme-form.tpl.php file into your theme, then change the id to something else, see if that fixes it.

catch, that's an interesting find on names and id's. It's either that or the UTF character encoding. See below.

I just tested on Drupal 5 and can't reproduce with IE7. I'm guessing it'd be the same with IE6. I would have noticed by now with all my testing with Drupal 5 since I *always* press return.

Here's the markup for the search box in Garland:

Drupal 5.2

<div class="block block-theme">
  <form action="/search/node"  method="post" id="search-theme-form">
    <div>
      <div id="search" class="container-inline">
        <div class="form-item">
          <input type="text" maxlength="128" name="search_theme_form_keys" id="edit-search-theme-form-keys"  size="15" value="" title="Enter the terms you wish to search for." class="form-text" />
        </div>
        <input type="submit" name="op" id="edit-submit" value="Search"  class="form-submit" />
        <input type="hidden" name="form_token" id="edit-search-theme-form-form-token" value="62dd66d0c47f979bf49e04eed6f82af1"  />
        <input type="hidden" name="form_id" id="edit-search-theme-form" value="search_theme_form"  />
      </div>
    </div>
  </form>
</div>

Drupal 6.dev

<div class="block block-theme">
  <form action="/drupal_6/"  accept-charset="UTF-8" method="post" id="search-theme-form">
    <div>
      <div id="search" class="container-inline">
        <div class="form-item" id="edit-search-theme-form-1-wrapper">
          <label for="edit-search-theme-form-1">Search this site: </label>
          <input type="text" maxlength="128" name="search_theme_form" id="edit-search-theme-form-1" size="15" value="" title="Enter the terms you wish to search for." class="form-text" />
        </div>
        <input type="submit" name="op" id="edit-submit" value="Search"  class="form-submit" />
        <input type="hidden" name="form_build_id" id="form-84e26f07111d412f280a085e76ab6d09" value="form-84e26f07111d412f280a085e76ab6d09"  />
        <input type="hidden" name="form_token" id="edit-search-theme-form-form-token" value="935ed03fe9a7b43d5899bdcd491bdeb1"  />
        <input type="hidden" name="form_id" id="edit-search-theme-form" value="search_theme_form"  />
      </div>
    </div>
  </form>
</div>

BTW, code formatting is broke. Had to encode this post.
[edit: code formatting is fixed. decoded post.]

I see search-theme-form-keys on the 5.x search module and no duplicates. Any reason we can't change 6.x to that?

Yeah, I think that'll fix it. I just checked with fiddler and string that's sent are these. Not sure I'm using it right but this looks like the culprit.

6:

search_theme_form=node&form_build_id=form-b5f934259cbcec2e4678a1dc056cec5c&form_token=0a0f5cc9cbc847da488ec4fe31b4aea6&form_id=search_theme_form

5:

search_theme_form_keys=node&form_token=3f0717a889efa65c2a759609fbd4a6ff&form_id=search_theme_form

Note the dupes. Catch nailed it.

Took another look and it was me who broke it during the template conversion. hehe

This workaround was originally in 5 but the comments were completely off on why it was there so I removed it. Here it is again with a more descriptive comment.

@ssb, you think this is important? How about testing it so it can go in.

ssb is right, same behaviour in IE7 patched as unpatched :| changing title since this clearly isn't the problem.

Well Damn.! I'm stumped. For anyone more knowledgeable here's the POST data gathered from Fiddler.

IE in Drupal 5 and FF 2 in Drupal 6 shown below for reference.

IE7 in Drupal 6 –the problem:

POST /drupal_6/ HTTP/1.1
Accept: image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, application/x-shockwave-flash, application/x-ms-application, application/x-ms-xbap, application/vnd.ms-xpsdocument, application/xaml+xml, */*
Referer: http://192.168.0.23/drupal_6/
Accept-Language: en-us
Content-Type: application/x-www-form-urlencoded
UA-CPU: x86
Accept-Encoding: gzip, deflate
User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 2.0.50727; .NET CLR 3.0.04506.648; .NET CLR 3.5.21022)
Proxy-Connection: Keep-Alive
Content-Length: 144
Host: 192.168.0.23
Pragma: no-cache
Cookie: SESS476c424d9bc18ba28917ba2fd704fa8f=9c0e3d423e54d29ac40128c75f19e40d; has_js=1
Authorization: Basic am9vbjpub29q

search_theme_form=node&form_build_id=form-89688d0cb7b51517cf4640f29ed26d20&form_token=7028b5ae7693126d490651d58de52eca&form_id=search_theme_form

IE7 in Drupal 5 –OK:

POST /search/node HTTP/1.1
Accept: image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, application/x-shockwave-flash, application/x-ms-application, application/x-ms-xbap, application/vnd.ms-xpsdocument, application/xaml+xml, */*
Referer: http://xxx/
Accept-Language: en-us
Content-Type: application/x-www-form-urlencoded
UA-CPU: x86
Accept-Encoding: gzip, deflate
User-Agent: Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1; .NET CLR 2.0.50727; .NET CLR 3.0.04506.648; .NET CLR 3.5.21022)
Proxy-Connection: Keep-Alive
Content-Length: 53
Host: xxx
Pragma: no-cache
Cookie: SESSd44812679b3f75e49385e2ff14c27883=bfd63533c6323bf947e409ebe872906b; __utma=214322325.642349984.1200099211.1200099211.1200605467.2; __utmz=214322325.1200099211.1.1.utmccn=(direct)|utmcsr=(direct)|utmcmd=(none); __utmb=214322325; __utmc=214322325

search_theme_form_keys=node&form_id=search_theme_form

FF2 in Drupal 6 -OK:

POST /drupal_6/ HTTP/1.1
Host: 192.168.0.23
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Proxy-Connection: keep-alive
Referer: http://192.168.0.23/drupal_6/
Cookie: has_js=1; SESS476c424d9bc18ba28917ba2fd704fa8f=a45f03c73c45b8dd4f88713f6f53837a
Authorization: Basic am9vbjpub29q
Content-Type: application/x-www-form-urlencoded
Content-Length: 154

search_theme_form=node&op=Search&form_build_id=form-1d27eed31eebaa96d1c77f5251064e21&form_token=7153f0aa20e4b45bf5cfd02a32a8cd2a&form_id=search_theme_form

I have to say although this doesn't actually break anything, it's going to lead to a lot of annoyance for a lot of visitors, and hence support requests and the rest, so would be good to get it fixed before release.

This is an idea: we can emulate the submit button asigning an action to the search form. The problem is that the post key value is not passed.

I did some Googling and apparently it turns out that IE has some "issues" when it comes to forms that have only one input field:
http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=903924&SiteID=1
http://kentreis.wordpress.com/2007/09/08/one-field-forms-and-ie-quirks/

So......... um, the following patch gives whole new meaning to the concept of "code needs work" ;) But it does fix the problem. (Actually, only 2/3 of the problem. When I tested with IE7, the search block works fine, as does the user search, but the main search form at /search/node is still broken - apparently the existence of the Advanced Search fieldset is giving it problems for some reason?)

As for how to fix this for real, I have no brilliant ideas, unfortunately.

Oh, also, the first link I posted claims that this issue only occurs when the form action is the same as the current page (I guess that would explain why it's not a problem in Drupal 5). So maybe something along the lines of @theborg's patch is the way to go?

You could use a hidden input type with conditional comments, that is not so ugly...

<!--[if IE 7]>
<input type="hidden" name="beNiceToIE7" />
<![endif]-->

or you can add some javascript to the one field that you do have
<input type='text' onkeydown='if(event.keyCode==13) this.submit()' />
(the "this" object in the example above would need to be a reference to the form itself).

ssb, nice find!

It's a typo in _form_builder_ie_cleanup.

Changed a !empty to empty and it works for me now, except I get an undefined index buttons error along with it.

So still needs work, but getting close now.

OK new patch fixes the notice by wrapping it in an isset. Might be a problem elsewhere, in which case this'd be a hack, but marking to CNR.

edit: cross posted with ssb. I didn't have any luck commenting out the unset() - this is my first ever look at form.inc though so any help much appreciated past this point!

Hm, if the form state buttons array's submit key is empty, how could its 0th index lead us to a button? The !empty() checked whether there are submit buttons on the form I guess.

+    if (empty($form_state['submitted']) && empty($form_state['buttons']['submit']) && empty($form_state['buttons']['button'])) {
+      $button = (isset($form_state['buttons']['submit'][0]));

This is much more general than search submission so retitling.

SSB's solution works great for me. After commenting out unset($form_state['buttons']); in forms.inc, all search fields (forms & boxes) work with Ie6/7 and other browsers.

Nice find!

Sorry! I just tried ssb's solution on a clean install and it works fine. Here's a patch which just removes the unset. I'm not sure if this should be moved elsewhere though, but leaving at needs review for now. Don't credit me if this gets in, just patchifying ssb's solution.

edit: ack! right patch this time.

Repatching ssb's and catch. Just added the unsetting to the cleanup function.

For some reason I tried this earlier but was unsuccessful. Anyway, tested this and it works great. Very small change in the end, so marking RTBC, since although I made patches during the issue, none of this final code has anything to do with me :)

It's still not working for me on the main search content page (i.e., at /search/node)... Anyone else see this too? For the search box and user searches it works great, though. Nice!

Oops.

If I expand the 'Advanced Search' fieldset (which has another submit button) it works patched or unpatched. Without that fieldset expanded, nothing.

The /search/node problem may be a separate issue, actually? (And probably not "criticial" since I just checked and it exists in Drupal 5.x as well.) The reason I say this is that it seems to exhibit different behavior. For the others, hitting Enter caused the page to reload without submitting the form. At /search/node, hitting Enter seems to literally do nothing...

#55, yes "literally nothing" is what I experienced, so yeah let's move that to a new issue once this gets in.

Sorry. That was bugging me. :P

I would test but I don't have IE.

Hm, this only removes the buttons in the IE case, otherwise it leaves the buttons there. Is this the intended behavior? Looks like this has some side effects on what is available in the form. Previously buttons were always unset after the clicked button was recognized. So does the solution to the bug at hand is that we need to keep the button information? It is unclear to me how does this work.

Ok Gábor, this has sense to me.
I've changed the unset to where it was before, but added a test on form submition to see if ie_cleanup has done the work on the buttons array.

Tested, and this works great. Can't see this having any side-effects so back to rtbc.

Sorry, but I am trying to understand what happens here. The "After handling the special IE case, we no longer need the buttons collection." does not say much in why we only remove it in certain cases. What happens with it if we don't remove it. Why do we need to keep it? What happens with it later?

Attached a patch which tries to be more to the point with a "If we already know which button was submitted, we don't need the buttons." comment. (Note that we already know we are after the IE case, so no need to repeat that). But this still does not explain why is it left otherwise. How is the clicked button identified in that case?

This bug took so much work to resolve, so why not document what we found, so later others will know better?

Indeed Gábor, now it has a better understanding, moving this to RTBC.

Edit: added some more explanation about the buttons array issue, and punctuation (Keith Smith).

Sounds much better. Thanks, now committed!

I just posted a new issue about the separate problem with /search/node (discussed in #53-#57 above):
http://drupal.org/node/214292

I am not comfortable implementing this patch. Not sure how other than copying and pasting what will read...

I know that // is comment, but what about the - or +, or @@,+++ etc...

Newbie needs some help.
-----------------------------------------------
I was writing code for my own search box querying the database for specific case and found that when I pressed enter it would send me back to the homepage. Pressing the button would run the search but enter key would not do it even if the button was highlighted. I then checked for the keycode 13 during a keypress event, then calling the same javascript, in this instance gotostore(), that pressing the button called. The event handler worked but calling the jscript, I didn't submit a form. The input is in a form and the button submits values, the code is below.

        <SCRIPT Language="JavaScript">

        nn=(document.layers)?true:false;
        ie=(document.all)?true:false;
        function keyDown(e) {
            var evt=(e)?e:(window.event)?window.event:null;
            if(evt){

             var key=(evt.charCode)?evt.charCode:((evt.keyCode)?evt.keyCode:((evt.which)?evt.which:0));

             if(key==13){

               gotostore();
             }
            }
        }




            function gotoStore()
            {
                search_wing = "http://localhost/sam/?q=node/54/"
		store_code = document.psc.psc_code.value;
                location.href = search_wing + store_code; 
            }
        </SCRIPT>

 <form name="psc">
            Filter  <input type="text" id="psc_code" value="Type PSC Code Filter"
                                  onclick="document.psc.psc_code.value = '';" >
            <input type="button" id="goto_store" title="Filter the list of PSC Codes" 
			value="filter" onclick="gotostore()" tabindex="1" />
        </form>

Any help or suggestions would be appreciated. I thought is was just a form submit issue, which i will do from the