Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13857 closed defect (fixed)

ColorPalette: makes crash of Browser of Android..

Reported by: Hikaru Tamura Owned by: bill
Priority: high Milestone: 1.7
Component: Dijit Version: 1.7.0b1
Keywords: ColorPalette Opener Cc: Atsushi Ono
Blocked By: Blocking:

Description

  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.

Change History (13)

comment:1 Changed 8 years ago by 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.

comment:2 Changed 8 years ago by ykami

Cc: bill added

comment:3 Changed 8 years ago by ykami

Cc: Atsushi Ono added; bill removed
Owner: changed from ykami to bill

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

comment:4 Changed 8 years ago by ykami

Component: DojoX MobileDijit

comment:5 Changed 8 years ago by 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.

comment:6 Changed 8 years ago by 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.)

comment:7 Changed 8 years ago by bill

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

comment:8 Changed 8 years ago by 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.

comment:9 Changed 8 years ago by 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.

comment:10 Changed 8 years ago by 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() 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]. At that time ColorPalette was composed of spans and divs.

Version 1, edited 8 years ago by bill (previous) (next) (diff)

comment:11 Changed 8 years ago by bill

Resolution: fixed
Status: newclosed

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.

comment:12 Changed 8 years ago by Hikaru Tamura

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

comment:13 Changed 8 years ago by bill

Milestone: tbd1.7
Summary: ColorPalette makes crash of Browser of Android..ColorPalette: makes crash of Browser of Android..

Great, glad it's working.

Note: See TracTickets for help on using tickets.