Bug 43872 – AX: isNativeCheckbox does not work as advertised

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[chris fleizach]

this method will return true for any InputElement. it does not account for someone doing something like

<input type='image' role="checkbox">

which would return true because it's an InputElement, but in reality is false


illustrated with this webpage
http://www.paciellogroup.com/blog/misc/ARIA/tristatecheck.html

[chris fleizach]

Created attachment 64158 [details]
Patch

[chris fleizach]

http://trac.webkit.org/changeset/65204

[Yuta Kitamura]

Hello,

I've found your r65204 produces a lot of test crashes (90+ tests) when it is merged in Chromium.

Change: http://src.chromium.org/viewvc/chrome?view=rev&revision=56014
Bot result: http://buildbot.jail.google.com/buildbot/chromium/builders/Webkit/builds/26081/steps/webkit_tests/logs/stdio
  (Please see the very bottom of the output)

To tell the truth, I really don't understand why these tests are crashing. I could not reproduce in my local environment, and the stack trace was totally unusable. Your patch may not be the fault.

However, we cannot leave this change as we don't want to see 90+ crashes when it is merged to Chromium. So, if there is no objections, I want to roll out r65204 in next hour or so.

Thank you for understanding. If you have any questions, please ask me.

[chris fleizach]

(In reply to comment #3)
> Hello,
> 
> I've found your r65204 produces a lot of test crashes (90+ tests) when it is merged in Chromium.
> 
> Change: http://src.chromium.org/viewvc/chrome?view=rev&revision=56014
> Bot result: http://buildbot.jail.google.com/buildbot/chromium/builders/Webkit/builds/26081/steps/webkit_tests/logs/stdio
>   (Please see the very bottom of the output)
> 
> To tell the truth, I really don't understand why these tests are crashing. I could not reproduce in my local environment, and the stack trace was totally unusable. Your patch may not be the fault.
> 
> However, we cannot leave this change as we don't want to see 90+ crashes when it is merged to Chromium. So, if there is no objections, I want to roll out r65204 in next hour or so.
> 
> Thank you for understanding. If you have any questions, please ask me.

As you can imagine, I am quite confused. Asking if there will be objections at 5AM obviously won't find too many objectors.

This URL gives me an error page as not found
http://buildbot.jail.google.com/buildbot/chromium/builders/Webkit/builds/26081/steps/webkit_tests/logs/stdio

and this page looks like the change just merged in a DEPS file
http://src.chromium.org/viewvc/chrome?view=rev&revision=56014

can you attach some actual crash logs. 

did these issues reproduce on the chromium build bots? 
i didn't hear anything about any problems all day long on the 12th.

did this actually fix your crashing issues? 

I don't see how this could affect anything besides accessibility related code. are you seeing crashes in accessibility code? 

i plan to roll this change back in at my earliest convenience. please include more evidence in the future.

[Yuta Kitamura]

(In reply to comment #4)
> (In reply to comment #3)
> > Hello,
> > 
> > I've found your r65204 produces a lot of test crashes (90+ tests) when it is merged in Chromium.
> > 
> > Change: http://src.chromium.org/viewvc/chrome?view=rev&revision=56014
> > Bot result: http://buildbot.jail.google.com/buildbot/chromium/builders/Webkit/builds/26081/steps/webkit_tests/logs/stdio
> >   (Please see the very bottom of the output)
> > 
> > To tell the truth, I really don't understand why these tests are crashing. I could not reproduce in my local environment, and the stack trace was totally unusable. Your patch may not be the fault.
> > 
> > However, we cannot leave this change as we don't want to see 90+ crashes when it is merged to Chromium. So, if there is no objections, I want to roll out r65204 in next hour or so.
> > 
> > Thank you for understanding. If you have any questions, please ask me.
> 
> As you can imagine, I am quite confused. Asking if there will be objections at 5AM obviously won't find too many objectors.
> 

I used almost all day of Friday (in Japan time) to track down this revision. My time was almost running out when I've figure out the revision. I hope you understand my situation.

Our repository was out of sync with WebKit for more than 24 hours (with 2 people working in different timezones) just because of this mysterious crashes...

> This URL gives me an error page as not found
> http://buildbot.jail.google.com/buildbot/chromium/builders/Webkit/builds/26081/steps/webkit_tests/logs/stdio
> 
> and this page looks like the change just merged in a DEPS file
> http://src.chromium.org/viewvc/chrome?view=rev&revision=56014
> 
> can you attach some actual crash logs. 

Sorry, I pasted the wrong address. The correct one is:

http://build.chromium.org/buildbot/waterfall/builders/Webkit/builds/26081/steps/webkit_tests/logs/stdio

However, it is not very useful, because stack trace is corrupted. Many media or video-related tests are crashing. A few CSS tests also crashes. It seems the set of crashing tests does not change.

> 
> did these issues reproduce on the chromium build bots? 
> i didn't hear anything about any problems all day long on the 12th.
> 
> did this actually fix your crashing issues? 

At least on our buildbots, it reliably reproduces. All the tries of WebKit merge beyond r65204 caused the same set of crashes.

Bad news is that I could not reproduce the issue in my local environment.

> 
> I don't see how this could affect anything besides accessibility related code. are you seeing crashes in accessibility code? 
> 
> i plan to roll this change back in at my earliest convenience. please include more evidence in the future.

I understand your confusion. I'm sorry for the fact I can't provide a concrete evidence. WebKit out-of-sync was pretty serious and there was no other way to avoid the crashes...

[chris fleizach]

I've re-added this patch. I believe it was rolled out erroneously. There's nothing in this change that would cause so many random failures. If the issue persists, I think someone needs to debug the build server (especially because the issue doesn't occur locally).

http://trac.webkit.org/changeset/65335