Clicking on links while accessibility is enabled does not render as expected
Created attachment 235154 [details] Patch
<rdar://problem/16826229>
Comment on attachment 235154 [details] Patch Attachment 235154 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4863039476072448 New failing tests: accessibility/plugin.html platform/mac/accessibility/aria-columnrowheaders.html platform/mac/accessibility/document-attributes.html accessibility/image-link.html platform/mac/accessibility/document-links.html accessibility/table-attributes.html accessibility/table-sections.html platform/mac/accessibility/internal-link-anchors.html accessibility/table-one-cell.html accessibility/table-detection.html platform/mac/accessibility/bounds-for-range.html accessibility/table-cells.html accessibility/image-map2.html accessibility/table-cell-spans.html accessibility/table-with-rules.html accessibility/transformed-element.html accessibility/internal-link-anchors2.html accessibility/lists.html
Created attachment 235159 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 235154 [details] Patch Attachment 235154 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6298681250676736 New failing tests: accessibility/plugin.html platform/mac/accessibility/aria-columnrowheaders.html platform/mac/accessibility/document-attributes.html accessibility/image-link.html platform/mac/accessibility/document-links.html accessibility/table-attributes.html accessibility/table-sections.html platform/mac/accessibility/internal-link-anchors.html accessibility/table-one-cell.html accessibility/table-detection.html platform/mac/accessibility/bounds-for-range.html accessibility/table-cells.html accessibility/image-map2.html accessibility/table-cell-spans.html accessibility/table-with-rules.html accessibility/transformed-element.html accessibility/internal-link-anchors2.html accessibility/lists.html
Created attachment 235163 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 235154 [details] Patch Attachment 235154 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4650046410719232 New failing tests: accessibility/plugin.html platform/mac/accessibility/aria-columnrowheaders.html platform/mac/accessibility/document-attributes.html accessibility/image-link.html platform/mac/accessibility/document-links.html accessibility/table-attributes.html accessibility/table-sections.html platform/mac/accessibility/internal-link-anchors.html accessibility/table-one-cell.html accessibility/table-detection.html platform/mac/accessibility/bounds-for-range.html accessibility/table-cells.html accessibility/image-map2.html media/track/track-long-word-container-sizing.html accessibility/table-cell-spans.html accessibility/table-with-rules.html accessibility/transformed-element.html accessibility/internal-link-anchors2.html accessibility/lists.html
Created attachment 235165 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 235154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235154&action=review > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:-2766 > - return nil; It looks like this is causing the test regressions. a bunch of tests except a nil. I think it might be good to have this in a different patch if possible, so that we can have a very focused patch just on updateBackingStore() issue
Chris: How would you feel if I updated the tests? Making sure that we adhere to incorrect behavior seems bad.
(In reply to comment #10) > Chris: How would you feel if I updated the tests? Making sure that we adhere to incorrect behavior seems bad. i think we need a separate bug to handle the issue from rdar://16826229. that's my only concern. thanks
Created attachment 235255 [details] Patch
Comment on attachment 235255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235255&action=review > Source/WebCore/ChangeLog:26 > + (WebCore::AccessibilityObject::updateBackingStore): Problem 3 above. There is no problem 3 above. > Source/WebCore/accessibility/AccessibilityObject.cpp:1440 > + if (!hasOneRef()) > + updateChildrenIfNecessary(); Why this hasOneRef() optimization? Is it for correctness or efficiency? Maybe it’s explained in the mysterious “problem 3” discussion?
Comment on attachment 235255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235255&action=review It’s gratuitous to include all those nullptr changes in this patch. Should just land them separately so this patch is small and to the point. >> Source/WebCore/ChangeLog:26 >> + (WebCore::AccessibilityObject::updateBackingStore): Problem 3 above. > > There is no problem 3 above. Oh, I see, it’s problem 2 above. >> Source/WebCore/accessibility/AccessibilityObject.cpp:1440 >> + updateChildrenIfNecessary(); > > Why this hasOneRef() optimization? Is it for correctness or efficiency? Maybe it’s explained in the mysterious “problem 3” discussion? Now that I understand “problem 2”, I believe firmly that the hasOneRef optimization is not helpful. I suggest you omit it for simplicity, even though it’s kind of neat to save the work. I don’t think we need to optimize this kind of unusual edge case, just prevent it from crashing.
Comment on attachment 235255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235255&action=review >> Source/WebCore/ChangeLog:26 >> + (WebCore::AccessibilityObject::updateBackingStore): Problem 3 above. > > There is no problem 3 above. Whoops. >> Source/WebCore/accessibility/AccessibilityObject.cpp:1440 >> + updateChildrenIfNecessary(); > > Why this hasOneRef() optimization? Is it for correctness or efficiency? Maybe it’s explained in the mysterious “problem 3” discussion? Efficiency. No reason to update children if we are going to die momentarily.
Comment on attachment 235255 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235255&action=review >>>> Source/WebCore/accessibility/AccessibilityObject.cpp:1440 >>>> + updateChildrenIfNecessary(); >>> >>> Why this hasOneRef() optimization? Is it for correctness or efficiency? Maybe it’s explained in the mysterious “problem 3” discussion? >> >> Now that I understand “problem 2”, I believe firmly that the hasOneRef optimization is not helpful. I suggest you omit it for simplicity, even though it’s kind of neat to save the work. I don’t think we need to optimize this kind of unusual edge case, just prevent it from crashing. > > Efficiency. No reason to update children if we are going to die momentarily. Alright, I'll remove the if
Created attachment 235257 [details] Patch
http://trac.webkit.org/changeset/171347