The JS popup menu that appeared was being drawn at the wrong location on the page. When hovering over the mouse over the menu bar (3rd horizontal bar, background color is white, popups initially take a while to load as they are loaded from server) the popup does not appear under the current menu item, but instead hovers on the far left. <rdar://problem/5720947>
Whoops, forgot to say, this regressed in r29425 -- eg. the activationimp patch
I'm working on a reduction.
Created attachment 18883 [details] Partial reduction The HTML on this site is insane. I've spent nearly an hour working on a reduction so far but still have a way to go. I'm going to put this aside for now and hope that someone else can finish it off.
Created attachment 18891 [details] Further reduction Here is a further reduction of this bug. The next thing to do is to merge in all of the dynamically included JS files. I'll do that soon, but first I need to shovel some snow.
Created attachment 18895 [details] Further reduction This is a further reduction of the bug. It now doesn't load any JS files dynamically. I think I can still clean it up a lot and make the reduction even smaller.
Probably a duplicate of bug 17015.
Created attachment 18896 [details] Further reduction Here is a further reduction of the bug. I found out what's going wrong. In the function calculateSumOffset(), the statement totalOffset += eval('item.' + offsetName); is not adding the value to totalOffset the first time through the loop. If you move the eval to a different line then it works fine.
I should also add that simply putting an eval('item.' + offsetName); statement before the offending line makes it work right.
I just ran into this same thing (as comment #7) in Version 3.0.4 (523.12.2). Given a form named "pops" and the name of an input of that form assigned to the variable "which", the following does not work (but should): var popUpText = ""; popUpText += eval("document.pops." + which + ".value"); alert(popUpText) However, the following modification does work: var popUpText = ""; var thisthing = eval("document.pops." + which + ".value") popUpText += thisthing; alert(popUpText) This even simpler modification also works (but limits its use): var popUpText = eval("document.pops." + which + ".value") alert(popUpText) So, it seems thatthis version of Webkit does not like combining "+=" with "eval". The Safari 3.04 does not have this problem, and neither did version 2.
Created attachment 18898 [details] Further reduction This is about as simple as it gets. Some notes: 1) It doesn't even matter if there is a lookup being done in the eval(). I just used a constant here and it still exhibits the problem. 2) The bug is exhibited in function scope but not global scope.
I found the likely cause of the bug. In the body of ReadModifyLocalVarNode::evaluate(), the slot is retrieved from the localStorage before calling valueForReadModifyAssignment(), which then would cause a tear-off by evaluating the call to eval. Similar problems (albeit more obscure ones) probably exist in the other callers of valueForReadModifyAssignment(). The most straightforward fix is to modify all of the callers of valueForReadModifyAssignment() to simply get the slot in which they will be storing the value one more time. A similar issue came up in the development of the tear-off patch, where using f.arguments could cause a tear-off that would require a slot to be retrieved again, and that's how I fixed it.
Created attachment 18900 [details] Fix Here's a patch that fixes this bug, as well as bug 16889 and bug 17015. I will add a nice big set of layout tests tomorrow, but I need to get some sleep first.
Comment on attachment 18900 [details] Fix I don't see the point of putting these re-gotten locations into the local variables. I would write this as: exec->localStorage()[m_index].value = v; and (*iter)->put(exec, m_ident, v); Also, I would suggest adding a comment at each of these call sites explaining why we can't use slot and base.
Created attachment 18925 [details] Proposed patch Here's the patch, along with tests. Darin, I agree with your comment so I changed the code. I also added a comment that should have been added to the original tear-off patch (I removed all of the copies of it with references to it, including the one referenced).
Comment on attachment 18925 [details] Proposed patch Add a newline to the end of the test and run sun-spider :D r=me
Created attachment 18926 [details] Revised proposed patch Oops. I always look at the diff in a text editor, but I didn't scroll to the last line. Here it is without the space I will try to run SunSpider later tonight, but I need to do some work first.
*** Bug 16889 has been marked as a duplicate of this bug. ***
Created attachment 18930 [details] Revised proposed patch I added more tests at Oliver's request.
Comment on attachment 18930 [details] Revised proposed patch r=me!
Landed r29997
*** Bug 17015 has been marked as a duplicate of this bug. ***