ColorPalette: makes crash of Browser of Android..

[Hikaru Tamura]

1. Open dojox/mobile/tests/test_Opener-ColorPalette?-async.html
2. Tap input.
3. Choose a color of Palette.

Browser crash occur (4. Push Done.)

I can reproduce it using Evo (Andoroid 2.2) and Galaxy Tab (2.2). I can't reproduce it by Xoom.

In Rev26444. I can reproduce it by one trial. In Rev26432. I can reproduce it by two or three times trial. In beta 5. I can reproduce it by many times trial.

[ykami]

• This is not an Opener issue, because I was able to reproduce it without Opener.
• To reproduce the problem, it looks we need both ColorPalette and TabBarButton. So far I cannot reproduce it with only one of them.

[ykami]

Bill, onoat will be explaining his findings (and perhaps suggestions) after a little more investigation. Please wait.

[Atsushi Ono]

It seems that changing focus inside setTimeout function (in _PaletteMixin#_onCellClick) causes the collision of focus timing on other elements, which results in browser crash on Android in some condition. When removing setTimeout in _PaletteMixin#_onCellClick, I could not reproduce the issue so far.

An another cause is that Android browser has the issue that absolute positioned elements disregard z-index and background elements sometimes steal focus. In dojox.mobile.Opener case, the cover element placed behind the color palette sometimes steal focus, which seems to lead to focus timing collision described above. ​http://code.google.com/p/android/issues/detail?id=6721

This Android browser issue has not been fixed so far, so removing setTimeout function except in IE might be the major candidate of solution if it does not have side effects in other browsers.

[bill]

Thanks for tracing that down. I'll try your suggestion of only using the timeout on IE. I wish that we could make the cells focusable by clicking but not be tab-navigable, so that we wouldn't need that cell.focus() call in the first place. But I suppose just setting tabIndex=-1 won't achieve that (although I'll test to make sure.)

[bill]

PS: what does "collision of focus timing on other elements" mean?

[Atsushi Ono]

what does "collision of focus timing on other elements" mean?

Sorry for a confusion. It means "focus on multiple elements almost at the same time". In dojox.mobile.Opener case, focus on a cell in Color Palette occurs almost at the same time with the unexpected focus on the cover behind the palette, which seems to cause browser crash on Android. It is difficult to get the accurate evidence because browser crashes at native code level, but I assumed so because the issue could not be reproduced when I removed the cell.focus() call in _PaletteMixin#_onCellClick or removed the cover element of dojox.mobile.Opener.

[Atsushi Ono]

Additionally, setTimeout can break focus event sequence, which seems to make "focus on multiple elements almost at the same time" easy to occur.

[bill]

The focus() call seems to only be needed on IE. I tried removing the focus() call, and moving the other calls outside of the setTimeout(), and ColorPalette seems to work fine on FF/win and safari mac.

On IE, the need for the setTimeout(), rather than a direct call to focus(), is dubious. Calendar's _onDayClick() also does a focus() call, but without a setTimeout(). ColorPalette's setTimeout() was added along with the focus() call in [20794], but I tried rolling back to [20794] and replacing the setTimeout() with a direct call to focus(), and the ColorPalette.html DOH test runs without errors on both IE6 and IE8.

[bill]

In [26654]:

Remove the setTimeout() around the focus() call in the onclick handler. It doesn't seem to be needed, even on IE, and it causes problems for mobile webkit. Fixes #13857 !strict.

[Hikaru Tamura]

I verified it. it works fine. Also, it seems response improvement.

[bill]

Great, glad it's working.