Bug 161454

Summary: Element.dataset.name incorrectly returns undefined
Product: WebKit Reporter: Robert Knight <robertknight>
Component: DOMAssignee: Chris Dumez <cdumez>
Severity: Normal CC: akiff, alakutin, cdumez, commit-queue, fpizlo, ggaren, justin, mark.lam, romanov4400, sam, shamsheev.js, sombragriselros, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 9   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=166835
Description Flags
dataset['features'] is returning undefined within function
Repro case
Patch none

Description Robert Knight 2016-08-31 13:36:17 PDT
The example at https://gist.github.com/robertknight/38784b71ddba63b1604b2c6244f216a1 walks the DOM and builds a map of name -> Element where the name is taken from the `data-ref` attribute. On Firefox, Chrome and Edge it successfully reads the `Element.dataset` values and displays `searchBucket, header, content`. On iOS Safari 9.3.5 (tested on an iPhone 5), Safari 9.1.2 (OS X 10.11) it very occasionally displays the correct values but usually reads all the `Element.dataset.ref` values as undefined.

The problem disappears with small modifications to the DOM structure, such as deleting a few of the `div` elements, other than the ones with `data-ref` attributes.
Comment 1 Robert Knight 2016-08-31 13:37:48 PDT
The issues is also very sensitive to how `Element.dataset` is accessed. The `findRefs()` function does not modify the DOM but commenting out the two unused calls to it also result in the problem disappearing.
Comment 2 Chris Dumez 2016-09-01 09:36:46 PDT
This seems to be working fine for me in Safari 10 so this may have been fixed already. Could you please try with Safari Technology Preview and confirm this is fixed?

-> https://developer.apple.com/safari/technology-preview/
Comment 3 aholt+bugzilla 2016-10-05 15:03:13 PDT
Created attachment 290752 [details]
dataset['features'] is returning undefined within function

This appears to still be happening within Safari 10.0, on both Mac OS X Sierra, as well as iOS 10.

With the previous version of Safari, we did not actually notice this happening, but on Safari 10.0, it is very common.

See screenshot.
Comment 4 Robert Knight 2016-10-09 05:49:08 PDT
With the original test case at https://gist.github.com/robertknight/38784b71ddba63b1604b2c6244f216a1 I can no longer reproduce the issue on my iPhone 5 with iOS 10.

However, by making a trivial modification to site.js so that line 18/19 (`findRefs(searchEl);`) is repeated several more times, I can reproduce the issue again with iOS 10.
Comment 5 Robert Knight 2016-10-09 05:59:48 PDT
This looks like it might be a JIT-related issue. Using the https://itunes.apple.com/us/app/webview-wkwebview-uiwebview/id928647773?mt=8 app, I can reproduce the problem in WKWebView and SafariViewController-powered web views but not UIWebView.
Comment 6 Justin Michael 2016-10-31 18:48:10 PDT
This bug is hitting me pretty hard in a web app I'm developing.  Interestingly, I've found that I can work around it by serializing the dataset into JSON and then parsing the JSON, which contains all the correct properties despite .property and ['property'] returning undefined.
Comment 7 Antonio Laguna 2017-04-05 22:24:19 PDT
This happened to me today. Still an issue. Workaround from Justin Michael did work.
Comment 8 Vladimir 2017-04-25 12:18:44 PDT
My workaround:
Comment 9 Chris Dumez 2017-04-25 12:55:23 PDT
This related to caching of properties. I was able to write a test case for this. I'll attach.
Comment 10 Chris Dumez 2017-04-25 12:55:42 PDT
Created attachment 308129 [details]
Repro case
Comment 11 Radar WebKit Bug Importer 2017-04-25 12:56:29 PDT
Comment 12 Chris Dumez 2017-04-25 12:57:34 PDT
JSDOMStringMap has the JSC::GetOwnPropertySlotIsImpure structure flag which is supposed to prevent caching AFAIK. Clearly, it is still caching though...
Comment 13 Mark Lam 2017-04-25 13:05:49 PDT
I wasn't able to reproduce the issue on WebKit trunk nor on STP.
Comment 14 Chris Dumez 2017-04-25 13:06:01 PDT
With my report case, I am able to reproduce with shipping Safari but not in the latest Safari Technology Preview. Therefore, I believe this was fixed recently. I'll bisect to try and figure out which change fixed it.
Comment 15 Chris Dumez 2017-04-25 13:38:25 PDT
This was fixed by Sam in http://trac.webkit.org/changeset/210667 as part of a large refactoring of our named property getters :)

I'll use this bug to land a layout test.
Comment 16 Chris Dumez 2017-04-25 13:48:04 PDT
Created attachment 308136 [details]
Comment 17 Mark Lam 2017-04-25 13:51:51 PDT
Comment on attachment 308136 [details]

Comment 18 WebKit Commit Bot 2017-04-25 14:41:35 PDT
Comment on attachment 308136 [details]

Clearing flags on attachment: 308136

Committed r215764: <http://trac.webkit.org/changeset/215764>
Comment 19 WebKit Commit Bot 2017-04-25 14:41:37 PDT
All reviewed patches have been landed.  Closing bug.