Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Reporter
	Description • 9 years ago

I'm working on bug 1128787. Its new test (dom/events/test/test_bug1128787-1.html) sometimes hits following assertion:

> [4712] ###!!! ASSERTION: Wrong in offset: 'Error', file a:\mozilla\mc-e\src\accessible\generic\HyperTextAccessible.cpp, line 1105
> #01: mozilla::a11y::HyperTextAccessible::CharBounds (a:\mozilla\mc-e\src\accessible\generic\hypertextaccessible.h:315)
> #02: mozilla::a11y::HyperTextAccessible::GetCaretRect (a:\mozilla\mc-e\src\accessible\generic\hypertextaccessible.cpp:1387)
> #03: mozilla::a11y::AccessibleWrap::UpdateSystemCaretFor (a:\mozilla\mc-e\src\accessible\windows\msaa\accessiblewrap.cpp:1389)
> #04: mozilla::a11y::AccessibleWrap::HandleAccEvent (a:\mozilla\mc-e\src\accessible\windows\msaa\accessiblewrap.cpp:1213)
> #05: mozilla::a11y::HyperTextAccessibleWrap::HandleAccEvent (a:\mozilla\mc-e\src\accessible\windows\msaa\hypertextaccessiblewrap.cpp:74)
> #06: nsEventShell::FireEvent (a:\mozilla\mc-e\src\accessible\base\nseventshell.cpp:36)
> #07: mozilla::a11y::SelectionManager::ProcessTextSelChangeEvent (a:\mozilla\mc-e\src\accessible\base\selectionmanager.cpp:169)
> #08: mozilla::a11y::EventQueue::ProcessEventQueue (a:\mozilla\mc-e\src\accessible\base\eventqueue.cpp:545)
> #09: mozilla::a11y::NotificationController::WillRefresh (a:\mozilla\mc-e\src\accessible\base\notificationcontroller.cpp:279)
> #10: nsRefreshDriver::Tick (a:\mozilla\mc-e\src\layout\base\nsrefreshdriver.cpp:1537)
> #11: mozilla::RefreshDriverTimer::TickDriver (a:\mozilla\mc-e\src\layout\base\nsrefreshdriver.cpp:199)
> #12: mozilla::RefreshDriverTimer::Tick (a:\mozilla\mc-e\src\layout\base\nsrefreshdriver.cpp:189)
> #13: mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers (a:\mozilla\mc-e\src\layout\base\nsrefreshdriver.cpp:441)
> #14: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver (a:\mozilla\mc-e\src\layout\base\nsrefreshdriver.cpp:376)
> #15: nsRunnableMethodImpl<void (__thiscall mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::*)(mozilla::TimeStamp),1,mozilla::TimeStamp>::Run (a:\mozilla\mc-e\fx-dbg\dist\include\nsthreadutils.h:669)
> #16: nsThread::ProcessNextEvent (a:\mozilla\mc-e\src\xpcom\threads\nsthread.cpp:866)
> #17: NS_ProcessNextEvent (a:\mozilla\mc-e\src\xpcom\glue\nsthreadutils.cpp:265)
> #18: mozilla::ipc::MessagePump::Run (a:\mozilla\mc-e\src\ipc\glue\messagepump.cpp:99)
> #19: MessageLoop::RunInternal (a:\mozilla\mc-e\src\ipc\chromium\src\base\message_loop.cc:233)
> #20: MessageLoop::RunHandler (a:\mozilla\mc-e\src\ipc\chromium\src\base\message_loop.cc:227)
> #21: MessageLoop::Run (a:\mozilla\mc-e\src\ipc\chromium\src\base\message_loop.cc:201)
> #22: nsBaseAppShell::Run (a:\mozilla\mc-e\src\widget\nsbaseappshell.cpp:166)
> #23: nsAppShell::Run (a:\mozilla\mc-e\src\widget\windows\nsappshell.cpp:180)
> #24: nsAppStartup::Run (a:\mozilla\mc-e\src\toolkit\components\startup\nsappstartup.cpp:282)
> #25: XREMain::XRE_mainRun (a:\mozilla\mc-e\src\toolkit\xre\nsapprunner.cpp:4171)
> #26: XREMain::XRE_main (a:\mozilla\mc-e\src\toolkit\xre\nsapprunner.cpp:4251)
> #27: XRE_main (a:\mozilla\mc-e\src\toolkit\xre\nsapprunner.cpp:4470)
> #28: do_main (a:\mozilla\mc-e\src\browser\app\nsbrowserapp.cpp:294)
> #29: NS_internal_main (a:\mozilla\mc-e\src\browser\app\nsbrowserapp.cpp:667)
> #30: wmain (a:\mozilla\mc-e\src\toolkit\xre\nswindowswmain.cpp:124)
> #31: __tmainCRTStartup (f:\dd\vctools\crt\crtw32\startup\crt0.c:255)
> #32: BaseThreadInitThunk[KERNEL32 +0x17c04]
> #33: RtlInitializeExceptionChain[ntdll +0x5b54f]
> #34: RtlInitializeExceptionChain[ntdll +0x5b51a]

test_bug1128787-1.html enables designMode from load event handler of window. Then, checking if focused input element loses focus and blur event is fired on it. After that (using executeSoon()), disabling designMode.

This assertion occurs only at running this test.

On the other hand, test_bug1128787-2.html which is almost same as test_bug11278787-1.html but it doesn't cause this assertion. The only difference between them is that test_bug1128787-2.html has <div contenteditable="true"/>. This means that the window is already editable at enabling designMode. This must be a big hint of this bug.

	Marco Zehe (:MarcoZ)
	Comment 1 • 9 years ago

Hi Alex, does this ring a bell?

	alexander :surkov (:asurkov)
	Comment 2 • 9 years ago

I think the focus was moved away when we processed selection change, caretOffset is -1, offset of CharBounds are -1, 0 what brings us into assertion when -1 magic offset is converted into character count. I think we should have something for caret move event processing similar to focus processing.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Reporter
	Comment 3 • 9 years ago

Hmm, I make the patch put off firing blur event later with newer patch. Then, I cannot reproduce this assertion. So, in this case, the assertion is caused by the blur event which should not be fired immediately after actual focus move at enabling designMode.
https://bugzilla.mozilla.org/attachment.cgi?oldid=8591695&action=interdiff&newid=8592840&headers=1

However, I saw same assertion with some tests at working on other bugs. Therefore, I believe that there is a bug that the method should not be called or the method should be ignore the cause with some conditions.

Should I keep this open? or should I close this as INVA?

	alexander :surkov (:asurkov)
	Comment 4 • 9 years ago

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #3)

> Should I keep this open? or should I close this as INVA?

It looks valid, at least stack shows plausible scenario how assertion may be triggered