Bug 23911

Summary: Exception thrown when opening a <select> pop-up that uses a web font
Product: WebKit Reporter: Eddy Nigg <eddy_nigg>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ddkilzer, mitz, vicki
Priority: P2 Keywords: HasReduction, InRadar
Version: 525.x (Safari 3.2)   
Hardware: Macintosh   
OS: OS X 10.5   
URL: https://www.startssl.com/?app=12
Attachments:
Description Flags
Test Case For onChange Functionality
none
Response for test case
none
test case
none
Use the system font for the pop-up if an NSFont cannot be obtained darin: review+

Description Eddy Nigg 2009-02-11 18:03:38 PST
Navigate to the URL https://www.startssl.com/?app=12 and click on register. In Safari the selects for country and states lock up. Code which could cause the problem is:

onchange="x_get_states_by_country_name(this.value,changeState);"

Which is AJAX call. The return function is:

function changeState(states) {
  stateElement = document.getElementById("State");
  stateElement.options.length = 0;
  var i = 0;

  for (var i in states){ 

    stateElement.options[i]=new Option(states[i], states[i], false, false);

  }
}
Comment 1 Mark Rowe (bdash) 2009-02-11 18:05:03 PST
<rdar://problem/6579204>
Comment 2 Eddy Nigg 2009-05-29 03:50:27 PDT
Is there any resolution coming for this bug or is there a known workaround for the JavaScript code?
Comment 3 Eddy Nigg 2009-11-16 16:23:26 PST
The JavaScript from comment 0 works with all browsers except Safari. I'm trying to have this bug somehow moved forward. If anything is need in order to help, please ping me.
Comment 4 Alexey Proskuryakov 2009-11-17 11:42:41 PST
One thing that could help is a reduction - a self-contained short example that shows the problem in action.
Comment 5 Eddy Nigg 2009-11-17 13:37:07 PST
Thanks for following up. The example can be seen at https://www.startssl.com/?app=11&action=regform

The code used is exactly as in comment 0 in particular:

stateElement.options[i]=new Option(states[i], states[i], false, false);

or a bit clearer:

element.options[1]=new Option(stringLabel, stringValue, false, false);

Incidentally I'm reading again through some JavaScript specs and it appears to me that the last false value might not be compatible (with some versions). Trying that right now...
Comment 6 Alexey Proskuryakov 2009-11-17 13:42:13 PST
I was talking about a reduced test case - a dozen lines or so of code total.

That's of course not a requirement for filing a bug, I only suggested that because you specifically asked how you could help.

> Incidentally I'm reading again through some JavaScript specs and it appears to
> me that the last false value might not be compatible (with some versions).

If that works in both IE and Firefox, we should strongly consider making it work in WebKit.
Comment 7 Eddy Nigg 2009-11-17 13:44:46 PST
I tried right now with  new Option(stringLabel, stringValue, false); and it doesn't work either with Safari.
Comment 8 Eddy Nigg 2009-11-17 13:48:38 PST
Some more information with this, which could be another issue: The SELECT fields have onchange="doSomething();" as well. Could this be the problem?
Comment 9 Eddy Nigg 2009-11-17 13:54:16 PST
(In reply to comment #6)
> I was talking about a reduced test case - a dozen lines or so of code total.
> 
> That's of course not a requirement for filing a bug, I only suggested that
> because you specifically asked how you could help.

Thank you, I understand...but doesn't the page source help? See https://www.startssl.com/?app=11&action=regform in particular line 367 and 868 with:

<select name="Country" onchange="x_get_states_by_country_name(this.value,changeState); x_get_phonecode_by_country_name(this.value,changePhoneCode);">

Not sure how much time I'd need to produce a test case mimicking the same behaviour... :S
Comment 10 Eddy Nigg 2009-11-17 18:51:57 PST
I'm now almost 100% certain that the cause for this behavior is the ONCHANGE in the SELECT field. Other SELECT fields which have onchange="doSomething();" lock up as well - without relation to the new Option() function.
Comment 11 David Kilzer (:ddkilzer) 2010-01-01 15:53:03 PST
This still occurs.  The "Register" link is now "Sign-up" apparently.

This issue is also mentioned on an Ars Technica article now:

<http://arstechnica.com/security/news/2009/12/how-to-get-set-with-a-secure-sertificate-for-free.ars>
Comment 12 Keith Anderson 2010-01-08 17:12:12 PST
Created attachment 46180 [details]
Test Case For onChange Functionality

I ran into this bug on their web site and their web developer pointed me to this bug report. I knocked up a test case that makes an attempt to recreate this issue with ONLY an onChange and AJAX request. I cannot duplicate the behavior, so I don't think that whatever is happening on that site is related to onChange.

There is a lot of document fiddling happening onLoad and otherwise, though - and something is preventing the popup menus from even being activated. Something else is going on here.

I've included as attachments my test case showing that XHR and onChange on a select element work fine.
Comment 13 Keith Anderson 2010-01-08 17:12:42 PST
Created attachment 46181 [details]
Response for test case

Put this next to the HTML test case - it functions as the AJAX response.
Comment 14 Alexey Proskuryakov 2010-01-11 10:19:25 PST
Created attachment 46287 [details]
test case

You may need to click the menu a couple of times before the problem reproduces with this test case.
Comment 15 Alexey Proskuryakov 2010-01-11 10:22:19 PST
This is caused by having an online @font-face for menu item text.

#0  0x94bf6dd4 in +[NSException raise:format:] ()
#1  0x9625b1f8 in -[NSCFDictionary setObject:forKey:] ()
#2  0x058f8ab3 in WebCore::PopupMenu::populate (this=0x1f8a6f60) at /Users/ap/Safari/OpenSource/WebCore/platform/mac/PopupMenuMac.mm:85
#3  0x058f8d03 in WebCore::PopupMenu::show (this=0x1f8a6f60, r=@0xbfffebb0, v=0xa904000, index=0) at /Users/ap/Safari/OpenSource/WebCore/platform/mac/PopupMenuMac.mm:107
#4  0x059ac7c2 in WebCore::RenderMenuList::showPopup (this=0x1f8a76bc) at /Users/ap/Safari/OpenSource/WebCore/rendering/RenderMenuList.cpp:290
#5  0x05a620af in WebCore::SelectElement::menuListDefaultEventHandler (data=@0x1f8a6a3c, element=0x1f8a69f0, event=0x203f4a60, htmlForm=0x0) at /Users/ap/Safari/OpenSource/WebCore/dom/SelectElement.cpp:660
Comment 16 Eddy Nigg 2010-01-11 11:48:03 PST
Hallelujah! That's easy to work around for us and hopefully there will be a solution for WebKit based browsers soon.
Comment 17 mitz 2010-01-20 12:23:07 PST
Created attachment 47052 [details]
Use the system font for the pop-up if an NSFont cannot be obtained

I tried to use the CTFontRef (which can be obtained for custom fonts), as it is toll-free bridged with NSFont, however the pop-up just used the system font. This will take care of the exception for now.
Comment 18 Darin Adler 2010-01-20 14:13:26 PST
Comment on attachment 47052 [details]
Use the system font for the pop-up if an NSFont cannot be obtained

> +                    font = style.font().weight() < FontWeightBold ? [NSFont systemFontOfSize:size] :[NSFont boldSystemFontOfSize:size];

Need a space after the : but before [NSFont].
Comment 19 mitz 2010-01-20 14:23:15 PST
Fixed in <http://trac.webkit.org/projects/webkit/changeset/53565>
Comment 20 Eddy Nigg 2010-01-20 14:44:39 PST
Thanks to all for working on this.