I believe this to be a bug. If I have '<div id="someElement"></div>' in my DOM, a 'global' variable called someElement is created, this is OK, as it is fairly standard behavior. Now, if I declare a global variable: 'var someElement;' This should now overshadow any global made with the element Id to make naming collisions with variables and elements not an issue. In Chrome, IE, Edge, FireFox, and Opera, my 'someElement' variable is 'undefined', as we would expect. In Safari... 'someElement' is actually equal to '<div id="someElement"></div>' instead of 'undefined'. This is not expected behavior and seems to be a bug when every other browser correctly handles this.
<rdar://problem/37927596>
Confirmed: http://jsbin.com/pigefuriso/edit?html,output
Replacing: var someElement; with var someElement = 1; fixes the issues. So we only ignore the local property if its value is undefined.
That breaks standard browser behavior. I initialize that variable to a widget later, so I count on it being undefined to only execute certain code if it is defined. The easiest solution is to change the element name to something unique, but it shouldn't have to be.
(In reply to tylerdah77 from comment #4) > That breaks standard browser behavior. > > I initialize that variable to a widget later, so I count on it being > undefined to only execute certain code if it is defined. > > The easiest solution is to change the element name to something unique, but > it shouldn't have to be. Oh, this was not meant as a workaround for you. Just giving us a hint as to where the bug could be.
Sorry, I thought you posted a 'solution' haha. Disregard.
Without looking closer I would guess this is because of the way that JSGlobalObject does getOwnPropertySlot and addGlobalVariable. If you define a variable it won't clear the existing global property, which I assume is the '<div id="someElement"></div>'. And when you do a getOwnPropertySlot on the global object it will see the old global property instead of shadowing. This is just a hunch, I can debug it more thoroughly later.
(In reply to Keith Miller from comment #7) > Without looking closer I would guess this is because of the way that > JSGlobalObject does getOwnPropertySlot and addGlobalVariable. If you define > a variable it won't clear the existing global property, which I assume is > the '<div id="someElement"></div>'. And when you do a getOwnPropertySlot on > the global object it will see the old global property instead of shadowing. > This is just a hunch, I can debug it more thoroughly later. This would be great, thanks Keith.
(In reply to Chris Dumez from comment #8) > (In reply to Keith Miller from comment #7) > > Without looking closer I would guess this is because of the way that > > JSGlobalObject does getOwnPropertySlot and addGlobalVariable. If you define > > a variable it won't clear the existing global property, which I assume is > > the '<div id="someElement"></div>'. And when you do a getOwnPropertySlot on > > the global object it will see the old global property instead of shadowing. > > This is just a hunch, I can debug it more thoroughly later. > > This would be great, thanks Keith. FYI, I do not see JSGlobalObject::addGlobalVar(const Identifier&) called. Therefore, JSGlobalObject::getOwnPropertySlot() is not able to find the variable in the symbol table.
(In reply to Chris Dumez from comment #9) > (In reply to Chris Dumez from comment #8) > > (In reply to Keith Miller from comment #7) > > > Without looking closer I would guess this is because of the way that > > > JSGlobalObject does getOwnPropertySlot and addGlobalVariable. If you define > > > a variable it won't clear the existing global property, which I assume is > > > the '<div id="someElement"></div>'. And when you do a getOwnPropertySlot on > > > the global object it will see the old global property instead of shadowing. > > > This is just a hunch, I can debug it more thoroughly later. > > > > This would be great, thanks Keith. > > FYI, I do not see JSGlobalObject::addGlobalVar(const Identifier&) called. > Therefore, JSGlobalObject::getOwnPropertySlot() is not able to find the > variable in the symbol table. I see now. JSGlobalObject::addVar() gets called but it is implemented like so: void addVar(ExecState* exec, const Identifier& propertyName) { if (!hasProperty(exec, propertyName)) addGlobalVar(propertyName); } hasProperty() returns true here (because of named property) so we do not call addGlobalVar().
Created attachment 335221 [details] WIP patch
Comment on attachment 335221 [details] WIP patch Attachment 335221 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/6845587 New failing tests: fast/forms/listbox-visible-size.html js/dom/var-declarations-shadowing.html
Created attachment 335230 [details] Archive of layout-test-results from ews101 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 335221 [details] WIP patch Attachment 335221 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/6845655 New failing tests: fast/forms/listbox-visible-size.html js/dom/var-declarations-shadowing.html
Created attachment 335233 [details] Archive of layout-test-results from ews104 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 335221 [details] WIP patch Attachment 335221 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/6845568 New failing tests: js/dom/var-declarations-shadowing.html fast/forms/listbox-visible-size.html
Created attachment 335237 [details] Archive of layout-test-results from ews115 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-sierra Platform: Mac OS X 10.12.6
Created attachment 335240 [details] Patch
Created attachment 335241 [details] Patch
Comment on attachment 335241 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335241&action=review > Source/JavaScriptCore/runtime/JSGlobalObject.h:514 > + if (!hasOwnProperty(exec, propertyName)) See spec at http://www.ecma-international.org/ecma-262/6.0/#sec-candeclareglobalvar
Comment on attachment 335241 [details] Patch Clearing flags on attachment: 335241 Committed r229451: <https://trac.webkit.org/changeset/229451>
All reviewed patches have been landed. Closing bug.