Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
845103003:
ASSERTION FAILED: !node || isElementOfType<const T>(*node) (Closed)
DescriptionASSERTION FAILED: !node || isElementOfType<const T>(*node)
BUG=447773
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188376
Patch Set 1 #Patch Set 2 : Adding proper check #
Total comments: 1
Patch Set 3 : Modified as per review comments #Messages
Total messages: 19 (6 generated)
ramya.v@samsung.com changed reviewers: + dmazzoni@chromium.org
PTAL! Thanks
I don't understand how it could ever be anything other than a HTMLTableElement. Unless we understand why, I don't think this change helps.
Wait, I just saw your note that you're able to reproduce the failure. Nice job! Can you add a logging statement and print out what type of node it is if it's *not* a table? Try something like this: fprintf(stderr, "The table node is actually this node: %s\n", node->nodeName().utf8().data());
On 2015/01/14 07:43:51, dmazzoni wrote: > Wait, I just saw your note that you're able to reproduce the failure. Nice job! > > Can you add a logging statement and print out what type of node it is if it's > *not* a table? > > Try something like this: > > fprintf(stderr, "The table node is actually this node: %s\n", > node->nodeName().utf8().data()); Its coming as BODY
On 2015/01/14 07:51:39, ramya.v wrote: > On 2015/01/14 07:43:51, dmazzoni wrote: > > Wait, I just saw your note that you're able to reproduce the failure. Nice > job! > > > > Can you add a logging statement and print out what type of node it is if it's > > *not* a table? > > > > Try something like this: > > > > fprintf(stderr, "The table node is actually this node: %s\n", > > node->nodeName().utf8().data()); > > Its coming as BODY Fascinating... I really would like to figure out what this fuzzed test is doing to make a RenderTable point at the BODY node, but in the meantime I think we can fix it almost as you patched it.
lgtm Oh, I think I know what's going on: probably <body style="display: table"> creates a RenderTable that doesn't correspond to a HTMLTableElement. Thanks for fixing this! https://codereview.chromium.org/845103003/diff/20001/Source/modules/accessibi... File Source/modules/accessibility/AXTable.cpp (right): https://codereview.chromium.org/845103003/diff/20001/Source/modules/accessibi... Source/modules/accessibility/AXTable.cpp:378: if (isHTMLTableElement(tableNode)) { Just return from this function if isHTMLTableElement returns false: if (!isHTMLTableElement(tableNode)) return false; We don't want to treat it like a table if it's not a table element.
On 2015/01/14 08:00:02, dmazzoni wrote: > lgtm > > Oh, I think I know what's going on: probably <body style="display: table"> > creates a RenderTable that doesn't correspond to a HTMLTableElement. > > Thanks for fixing this! > > https://codereview.chromium.org/845103003/diff/20001/Source/modules/accessibi... > File Source/modules/accessibility/AXTable.cpp (right): > > https://codereview.chromium.org/845103003/diff/20001/Source/modules/accessibi... > Source/modules/accessibility/AXTable.cpp:378: if (isHTMLTableElement(tableNode)) > { > Just return from this function if isHTMLTableElement returns false: > > if (!isHTMLTableElement(tableNode)) > return false; > > We don't want to treat it like a table if it's not a table element. Updated as per review comments.
The CQ bit was checked by ramya.v@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/845103003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/4...)
The CQ bit was checked by ramya.v@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/845103003/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/4...)
The CQ bit was checked by ramya.v@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/845103003/40001
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://src.chromium.org/viewvc/blink?view=rev&revision=188376 |
