WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
93485
[Microdata] PropertyNodeList cache should be invalidated on id attribute change
https://bugs.webkit.org/show_bug.cgi?id=93485
Summary
[Microdata] PropertyNodeList cache should be invalidated on id attribute change
Arko Saha
Reported
2012-08-08 07:56:48 PDT
PropertyNodeList cache is not getting invalidated when id attribute of an element changed/modified. Test: var parEl = makeEl('div',{},'<div itemprop="foo"><div itemprop="bar"></div></div><div itemscope itemref="id1"></div>'); var testEl = parEl.childNodes[1]; assert_equals( testEl.properties.namedItem('foo').length, 0, 'foo length (before test)' ); parEl.firstChild.id = 'id1'; assert_equals( testEl.properties.namedItem('foo').length, 1, 'foo length after id is created' ); Expected: Pass Actual: Fail- assert_equals: foo length after id is created expected 1 but got 0
Attachments
Patch
(11.03 KB, patch)
2012-08-08 08:17 PDT
,
Arko Saha
rniwa
: review+
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Arko Saha
Comment 1
2012-08-08 08:17:46 PDT
Created
attachment 157222
[details]
Patch
WebKit Review Bot
Comment 2
2012-08-09 00:29:21 PDT
Comment on
attachment 157222
[details]
Patch Rejecting
attachment 157222
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ore/ChangeLog CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog Failed to merge in the changes. Patch failed at 0001 Add optional debug logging for tiled scrolling When you have resolved this problem run "git rebase --continue". If you would prefer to skip this patch, instead run "git rebase --skip". To restore the original branch and stop rebasing run "git rebase --abort". rebase refs/remotes/origin/master: command returned error: 1 Died at Tools/Scripts/update-webkit line 164. Full output:
http://queues.webkit.org/results/13455807
Arko Saha
Comment 3
2012-08-09 00:48:21 PDT
Committed
r125159
: <
http://trac.webkit.org/changeset/125159
>
Sudarsana Nagineni (babu)
Comment 4
2012-08-10 06:24:29 PDT
I suspect this is caused an assertion on EFL debug bot. Could you check it, please?
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/4076
Stack Trace:
http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug/r125160%20%284076%29/fast/flexbox/001-crash-log.txt
These tests are flaky on the bot with the same stack trace. fast/files/apply-blob-url-to-img.html = CRASH fast/eventsource/eventsource-attribute-listeners.html = CRASH fast/flexbox/001.html = CRASH fast/flexbox/002.html = CRASH
Arko Saha
Comment 5
2012-08-10 09:06:42 PDT
(In reply to
comment #4
)
> I suspect this is caused an assertion on EFL debug bot. Could you check it, please? > >
http://build.webkit.org/builders/EFL%20Linux%2064-bit%20Debug/builds/4076
> > Stack Trace: >
http://build.webkit.org/results/EFL%20Linux%2064-bit%20Debug/r125160%20%284076%29/fast/flexbox/001-crash-log.txt
> > These tests are flaky on the bot with the same stack trace. > > fast/files/apply-blob-url-to-img.html = CRASH > fast/eventsource/eventsource-attribute-listeners.html = CRASH > fast/flexbox/001.html = CRASH > fast/flexbox/002.html = CRASH
Thanks for reporting the issue. I will analyze the root cause of the crash and update my analysis as soon as possible.
Ryosuke Niwa
Comment 6
2012-08-10 10:41:45 PDT
Comment on
attachment 157222
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=157222&action=review
> Source/WebCore/dom/DynamicNodeList.h:71 > + ALWAYS_INLINE bool isRootedAtDocument() const { return m_rootType == NodeListIsRootedAtDocument || ownerNodeHasItemRefAttribute(); }
Oops, this needs to just check if m_rootType is NodeListIsRootedAtDocumentIfOwnerHasItemrefAttr, not that it's currently rooted at the document.
Arko Saha
Comment 7
2012-08-10 11:02:48 PDT
(In reply to
comment #6
)
> (From update of
attachment 157222
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=157222&action=review
> > > Source/WebCore/dom/DynamicNodeList.h:71 > > + ALWAYS_INLINE bool isRootedAtDocument() const { return m_rootType == NodeListIsRootedAtDocument || ownerNodeHasItemRefAttribute(); } > > Oops, this needs to just check if m_rootType is NodeListIsRootedAtDocumentIfOwnerHasItemrefAttr, not that it's currently rooted at the document.
Yes you are right. In Document::registerNodeListCache() it was not adding PropertyNodeList cache to m_listsInvalidatedAtDocument as it's currently not rooted at the document. Where in Document::unregisterNodeListCache() it was trying to remove PropertyNodeList cache from m_listsInvalidatedAtDocument. Hence it caused the assertion failure. We should modify the check then.
Ryosuke Niwa
Comment 8
2012-08-10 11:12:55 PDT
Could you file a new bug and post a patch there?
Arko Saha
Comment 9
2012-08-10 11:32:24 PDT
(In reply to
comment #8
)
> Could you file a new bug and post a patch there?
Sure, filed bug :
https://bugs.webkit.org/show_bug.cgi?id=93729
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug