Closed Bug 1295354 Opened 8 years ago Closed 8 years ago

MOZ_ASSERT(nodePosition.mOffset) on e10s while sending a message with an emoticon on hangouts.google.com

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: emilio, Assigned: masayuki)

References

()

Details

(Keywords: regression)

Attachments

(2 files)

I hit this on current nightly, and it causes my computer to freeze due to a OOM. I grabbed a debug build and hit this assertion. STR: 1 - Open hangouts.google.com, and log in (sorry!). 2 - Open an existing text conversation, and type a message with an emoticon, such as |:)|. 3 - See the content process freeze as it takes more and more memory. It's not reproducible a 100% of the time, but it's pretty frequently (50+% of them). I've only being able to reproduce it in conversations that were inactive for a few minutes (where the avatar of the other person is at the bottom of the conversation, not a message of yours). Nightly crash (killed manually before OOM): https://crash-stats.mozilla.com/report/index/97a3772d-5d69-4d46-9a35-bf1fa2160815 Linux GTK, just in case it's widget-dependent. Backtrace in a debug build: #6 0x00007f9d407b5051 in mozilla::ContentEventHandler::GetLastFrameInRangeForTextRect (this=0x7ffeb40a6af0, aRange=0x7f9d031e0a00) at /home/emilio/projects/moz/gecko-4/dom/events/ContentEventHandler.cpp:1618 1618 MOZ_ASSERT(nodePosition.mOffset); (gdb) Quit (gdb) bt #0 0x00007f9d3bd32ffd in nanosleep () at /usr/lib/libc.so.6 #1 0x00007f9d3bd32f4a in sleep () at /usr/lib/libc.so.6 #2 0x00007f9d425a6b34 in ah_crap_handler(int) (signum=11) at /home/emilio/projects/moz/gecko-4/toolkit/xre/nsSigHandlers.cpp:103 #3 0x00007f9d425a6b7f in child_ah_crap_handler(int) (signum=11) at /home/emilio/projects/moz/gecko-4/toolkit/xre/nsSigHandlers.cpp:115 #4 0x00007f9d43757fea in AsmJSFaultHandler<(Signal)0>(int, siginfo_t*, void*) (signum=11, info=0x7ffeb40a5c30, context=0x7ffeb40a5b00) at /home/emilio/projects/moz/gecko-4/js/src/asmjs/WasmSignalHandlers.cpp:1211 #5 0x00007f9d4759c080 in <signal handler called> () at /usr/lib/libpthread.so.0 #6 0x00007f9d407b5051 in mozilla::ContentEventHandler::GetLastFrameInRangeForTextRect(nsRange*) (this=0x7ffeb40a6af0, aRange=0x7f9d031e0a00) at /home/emilio/projects/moz/gecko-4/dom/events/ContentEventHandler.cpp:1618 #7 0x00007f9d407b7151 in mozilla::ContentEventHandler::OnQueryTextRect(mozilla::WidgetQueryContentEvent*) (this=0x7ffeb40a6af0, aEvent=0x7ffeb40a67a0) at /home/emilio/projects/moz/gecko-4/dom/events/ContentEventHandler.cpp:2143 #8 0x00007f9d407b6ac7 in mozilla::ContentEventHandler::OnQueryTextRectArray(mozilla::WidgetQueryContentEvent*) (this=0x7ffeb40a6af0, aEvent=0x7ffeb40a79b0) at /home/emilio/projects/moz/gecko-4/dom/events/ContentEventHandler.cpp:2067 #9 0x00007f9d407b385f in mozilla::ContentEventHandler::HandleQueryContentEvent(mozilla::WidgetQueryContentEvent*) (this=0x7ffeb40a6af0, aEvent=0x7ffeb40a79b0) at /home/emilio/projects/moz/gecko-4/dom/events/ContentEventHandler.cpp:1248 #10 0x00007f9d407e6103 in mozilla::IMEContentObserver::HandleQueryContentEvent(mozilla::WidgetQueryContentEvent*) (this=0x7f9d03692bc0, aEvent=0x7ffeb40a79b0) at /home/emilio/projects/moz/gecko-4/dom/events/IMEContentObserver.cpp:775 #11 0x00007f9d40798436 in mozilla::EventStateManager::HandleQueryContentEvent(mozilla::WidgetQueryContentEvent*) (this=0x7f9d0b8b8540, aEvent=0x7ffeb40a79b0) at /home/emilio/projects/moz/gecko-4/dom/events/EventStateManager.cpp:871 #12 0x00007f9d40797b3e in mozilla::EventStateManager::PreHandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIFrame*, nsIContent*, nsEventStatus*) (this=0x7f9d0b8b8540, aPresContext=0x7f9d05bca800, aEvent=0x7ffeb40a79b0, aTargetFrame=0x7f9d0b70e768, aTargetContent=0x7f9d11d53ca0, aStatus=0x7ffeb40a78cc) at /home/emilio/projects/moz/gecko-4/dom/events/EventStateManager.cpp:595 #13 0x00007f9d419f2d8f in PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) (this=0x7f9d04a3c000, aEvent=0x7ffeb40a79b0, aStatus=0x7ffeb40a78cc, aIsHandlingNativeEvent=true) at /home/emilio/projects/moz/gecko-4/layout/base/nsPresShell.cpp:8495 #14 0x00007f9d419f218f in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) (this=0x7f9d04a3c000, aFrame=0x7f9d05bf8968, aEvent=0x7ffeb40a79b0, aDontRetargetEvents=true, aEventStatus=0x7ffeb40a78cc, aTargetContent=0x0) at /home/emilio/projects/moz/gecko-4/layout/base/nsPresShell.cpp:8220 #15 0x00007f9d419f036b in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) (this=0x7f9d116d6800, aFrame=0x7f9d11b14b68, aEvent=0x7ffeb40a79b0, aDontRetargetEvents=false, aEventStatus=0x7ffeb40a78cc, aTargetContent=0x0) at /home/emilio/projects/moz/gecko-4/layout/base/nsPresShell.cpp:7723 #16 0x00007f9d41502d31 in nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) (this=0x7f9d0b2c4ac0, aEvent=0x7ffeb40a79b0, aView=0x7f9d03c12980, aStatus=0x7ffeb40a78cc) at /home/emilio/projects/moz/gecko-4/view/nsViewManager.cpp:816 #17 0x00007f9d414ff270 in nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) (this=0x7f9d03c12980, aEvent=0x7ffeb40a79b0, aUseAttachedEvents=false) at /home/emilio/projects/moz/gecko-4/view/nsView.cpp:1121 #18 0x00007f9d4152f7ca in mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) (this=0x7f9d1d64f400, event=0x7ffeb40a79b0, aStatus=@0x7ffeb40a7acc: nsEventStatus_eIgnore) at /home/emilio/projects/moz/gecko-4/widget/PuppetWidget.cpp:350 #19 0x00007f9d4151d3ea in mozilla::ContentCacheInChild::QueryCharRectArray(nsIWidget*, unsigned int, unsigned int, nsTArray<mozilla::gfx::IntRectTyped<mozilla::LayoutDevicePixel> >&) const (this=0x7f9d1d64f5e8, aWidget=0x7f9d1d64f400, aOffset=0, aLength=2, aCharRectArray=...) at /home/emilio/projects/moz/gecko-4/widget/ContentCache.cpp:309 #20 0x00007f9d4151d8ab in mozilla::ContentCacheInChild::CacheTextRects(nsIWidget*, mozilla::widget::IMENotification const*) (this=0x7f9d1d64f5e8, aWidget=0x7f9d1d64f400, aNotification=0x7ffeb40a8400) at /home/emilio/projects/moz/gecko-4/widget/ContentCache.cpp:371 #21 0x00007f9d4151c946 in mozilla::ContentCacheInChild::CacheSelection(nsIWidget*, mozilla::widget::IMENotification const*) (this=0x7f9d1d64f5e8, aWidget=0x7f9d1d64f400, aNotification=0x7ffeb40a8400) at /home/emilio/projects/moz/gecko-4/widget/ContentCache.cpp:182 #22 0x00007f9d4151d17d in mozilla::ContentCacheInChild::CacheText(nsIWidget*, mozilla::widget::IMENotification const*) (this=0x7f9d1d64f5e8, aWidget=0x7f9d1d64f400, aNotification=0x7ffeb40a8400) at /home/emilio/projects/moz/gecko-4/widget/ContentCache.cpp:271 #23 0x00007f9d41530b71 in mozilla::widget::PuppetWidget::NotifyIMEOfTextChange(mozilla::widget::IMENotification const&) (this=0x7f9d1d64f400, aIMENotification=...) at /home/emilio/projects/moz/gecko-4/widget/PuppetWidget.cpp:850 #24 0x00007f9d415304bd in mozilla::widget::PuppetWidget::NotifyIMEInternal(mozilla::widget::IMENotification const&) (this=0x7f9d1d64f400, aIMENotification=...) at /home/emilio/projects/moz/gecko-4/widget/PuppetWidget.cpp:639 #25 0x00007f9d4150f713 in nsBaseWidget::NotifyIME(mozilla::widget::IMENotification const&) (this=0x7f9d1d64f400, aIMENotification=...) at /home/emilio/projects/moz/gecko-4/widget/nsBaseWidget.cpp:1854 #26 0x00007f9d407eeb9d in mozilla::IMEStateManager::NotifyIME(mozilla::widget::IMENotification const&, nsIWidget*, bool) (aNotification=..., aWidget=0x7f9d1d64f400, aOriginIsRemote=false) at /home/emilio/projects/moz/gecko-4/dom/events/IMEStateManager.cpp:1426 ---Type <return> to continue, or q <return> to quit--- #27 0x00007f9d407e9a05 in mozilla::IMEContentObserver::IMENotificationSender::SendTextChange() (this=0x7f9d0328be20) at /home/emilio/projects/moz/gecko-4/dom/events/IMEContentObserver.cpp:1769 #28 0x00007f9d407e8bc3 in mozilla::IMEContentObserver::IMENotificationSender::Run() (this=0x7f9d0328be20) at /home/emilio/projects/moz/gecko-4/dom/events/IMEContentObserver.cpp:1556 #29 0x00007f9d407e85d3 in mozilla::IMEContentObserver::TryToFlushPendingNotifications() (this=0x7f9d03692bc0) at /home/emilio/projects/moz/gecko-4/dom/events/IMEContentObserver.cpp:1437 #30 0x00007f9d40797454 in mozilla::EventStateManager::TryToFlushPendingNotificationsToIME() (this=0x7f9d0b8b8540) at /home/emilio/projects/moz/gecko-4/dom/events/EventStateManager.cpp:479 #31 0x00007f9d419f2fa5 in PresShell::HandleEventInternal(mozilla::WidgetEvent*, nsEventStatus*, bool) (this=0x7f9d04a3c000, aEvent=0x7ffeb40a9160, aStatus=0x7ffeb40a904c, aIsHandlingNativeEvent=true) at /home/emilio/projects/moz/gecko-4/layout/base/nsPresShell.cpp:8530 #32 0x00007f9d419f218f in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) (this=0x7f9d04a3c000, aFrame=0x7f9d05bf8968, aEvent=0x7ffeb40a9160, aDontRetargetEvents=true, aEventStatus=0x7ffeb40a904c, aTargetContent=0x0) at /home/emilio/projects/moz/gecko-4/layout/base/nsPresShell.cpp:8220 #33 0x00007f9d419f036b in PresShell::HandleEvent(nsIFrame*, mozilla::WidgetGUIEvent*, bool, nsEventStatus*, nsIContent**) (this=0x7f9d116d6800, aFrame=0x7f9d11b14b68, aEvent=0x7ffeb40a9160, aDontRetargetEvents=false, aEventStatus=0x7ffeb40a904c, aTargetContent=0x0) at /home/emilio/projects/moz/gecko-4/layout/base/nsPresShell.cpp:7723 #34 0x00007f9d41502d31 in nsViewManager::DispatchEvent(mozilla::WidgetGUIEvent*, nsView*, nsEventStatus*) (this=0x7f9d0b2c4ac0, aEvent=0x7ffeb40a9160, aView=0x7f9d03c12980, aStatus=0x7ffeb40a904c) at /home/emilio/projects/moz/gecko-4/view/nsViewManager.cpp:816 #35 0x00007f9d414ff270 in nsView::HandleEvent(mozilla::WidgetGUIEvent*, bool) (this=0x7f9d03c12980, aEvent=0x7ffeb40a9160, aUseAttachedEvents=false) at /home/emilio/projects/moz/gecko-4/view/nsView.cpp:1121 #36 0x00007f9d4152f7ca in mozilla::widget::PuppetWidget::DispatchEvent(mozilla::WidgetGUIEvent*, nsEventStatus&) (this=0x7f9d1d64f400, event=0x7ffeb40a9160, aStatus=@0x7ffeb40a912c: nsEventStatus_eIgnore) at /home/emilio/projects/moz/gecko-4/widget/PuppetWidget.cpp:350 #37 0x00007f9d3f0f4e1f in mozilla::layers::APZCCallbackHelper::DispatchWidgetEvent(mozilla::WidgetGUIEvent&) (aEvent=...) at /home/emilio/projects/moz/gecko-4/gfx/layers/apz/util/APZCCallbackHelper.cpp:463 #38 0x00007f9d4117c78e in mozilla::dom::TabChild::RecvRealKeyEvent(mozilla::WidgetKeyboardEvent const&, mozilla::dom::MaybeNativeKeyBinding const&) (this=0x7f9d1d64ec00, event=..., aBindings=...) at /home/emilio/projects/moz/gecko-4/dom/ipc/TabChild.cpp:2107 #39 0x00007f9d3e861ae9 in mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&) (this=0x7f9d1d64ec60, msg__=...) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/ipc/ipdl/PBrowserChild.cpp:3897 #40 0x00007f9d3e96d637 in mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&) (this=0x7f9d2fc81020, msg__=...) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/ipc/ipdl/PContentChild.cpp:7438 #41 0x00007f9d3e28bbbc in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) (this=0x7f9d2fc81088, aMsg=...) at /home/emilio/projects/moz/gecko-4/ipc/glue/MessageChannel.cpp:1662 #42 0x00007f9d3e28b6c0 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) (this=0x7f9d2fc81088, aMsg=<unknown type in /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/dist/bin/libxul.so, CU 0x2167172, DIE 0x21e3b49>) at /home/emilio/projects/moz/gecko-4/ipc/glue/MessageChannel.cpp:1600 #43 0x00007f9d3e28b40d in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() (this=0x7f9d2fc81088) at /home/emilio/projects/moz/gecko-4/ipc/glue/MessageChannel.cpp:1567 #44 0x00007f9d3e2a8e28 in mozilla::detail::RunnableMethodArguments<>::applyImpl<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()>(mozilla::ipc::MessageChannel*, bool (mozilla::ipc::MessageChannel::*)(), mozilla::Tuple<>&, mozilla::IndexSequence<>) (o=0x7f9d2fc81088, m=(bool (mozilla::ipc::MessageChannel::*)(mozilla::ipc::MessageChannel * const)) 0x7f9d3e28b2ac <mozilla::ipc::MessageChannel::OnMaybeDequeueOne()>, args=...) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/dist/include/nsThreadUtils.h:729 #45 0x00007f9d3e2a8ad8 in mozilla::detail::RunnableMethodArguments<>::apply<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()>(mozilla::ipc::MessageChannel*, bool (mozilla::ipc::MessageChannel::*)()) (this=0x7f9d2fc41b38, o=0x7f9d2fc81088, m=(bool (mozilla::ipc::MessageChannel::*)(mozilla::ipc::MessageChannel * const)) 0x7f9d3e28b2ac <mozilla::ipc::MessageChannel::OnMaybeDequeueOne()>) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/dist/include/nsThreadUtils.h:736 #46 0x00007f9d3e2a85bd in mozilla::detail::RunnableMethodImpl<bool (mozilla::ipc::MessageChannel::*)(), false, true>::Run() (this=0x7f9d2fc41b00) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/dist/include/nsThreadUtils.h:764 #47 0x00007f9d3e297075 in mozilla::ipc::MessageChannel::RefCountedTask::Run() (this=0x7f9d2fc4b6d0) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/dist/include/mozilla/ipc/MessageChannel.h:545 #48 0x00007f9d3e297284 in mozilla::ipc::MessageChannel::DequeueTask::Run() (this=0x7f9d0315f7f0) at /home/emilio/projects/moz/gecko-4/obj-x86_64-pc-linux-gnu/dist/include/mozilla/ipc/MessageChannel.h:564 #49 0x00007f9d3daf6e30 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f9d2eccc100, aMayWait=true, aResult=0x7ffeb40acc4f) at /home/emilio/projects/moz/gecko-4/xpcom/threads/nsThread.cpp:1058 #50 0x00007f9d3db5fe50 in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x7f9d2eccc100, aMayWait=true) at /home/emilio/projects/moz/gecko-4/xpcom/glue/nsThreadUtils.cpp:290 #51 0x00007f9d3e28f95a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0x7f9d2fc83290, aDelegate=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/glue/MessagePump.cpp:124 #52 0x00007f9d3e290268 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (this=0x7f9d2fc83290, aDelegate=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/glue/MessagePump.cpp:301 ---Type <return> to continue, or q <return> to quit--- #53 0x00007f9d3e1ff58f in MessageLoop::RunInternal() (this=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:232 #54 0x00007f9d3e1ff522 in MessageLoop::RunHandler() (this=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:225 #55 0x00007f9d3e1ff4fb in MessageLoop::Run() (this=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:205 #56 0x00007f9d41543460 in nsBaseAppShell::Run() (this=0x7f9d2253feb0) at /home/emilio/projects/moz/gecko-4/widget/nsBaseAppShell.cpp:156 #57 0x00007f9d425a26b2 in XRE_RunAppShell() () at /home/emilio/projects/moz/gecko-4/toolkit/xre/nsEmbedFunctions.cpp:846 #58 0x00007f9d3e2900fd in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) (this=0x7f9d2fc83290, aDelegate=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/glue/MessagePump.cpp:269 #59 0x00007f9d3e1ff58f in MessageLoop::RunInternal() (this=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:232 #60 0x00007f9d3e1ff522 in MessageLoop::RunHandler() (this=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:225 #61 0x00007f9d3e1ff4fb in MessageLoop::Run() (this=0x7ffeb40acee0) at /home/emilio/projects/moz/gecko-4/ipc/chromium/src/base/message_loop.cc:205 #62 0x00007f9d425a212a in XRE_InitChildProcess(int, char**, XREChildData const*) (aArgc=3, aArgv=0x7ffeb40ad358, aChildData=0x7ffeb40ad200) at /home/emilio/projects/moz/gecko-4/toolkit/xre/nsEmbedFunctions.cpp:676 #63 0x00000000004234cf in content_process_main(int, char**) (argc=5, argv=0x7ffeb40ad358) at /home/emilio/projects/moz/gecko-4/ipc/app/../contentproc/plugin-container.cpp:197 #64 0x0000000000423598 in main(int, char**) (argc=6, argv=0x7ffeb40ad358) at /home/emilio/projects/moz/gecko-4/ipc/app/MozillaRuntimeMain.cpp:18 I haven't investigated that much on why this assertion lead to OOM, probably due to the underflowed offset. Let me know if I can provide more info.
Needinfoing Masayuki Nakano, that was according to the git blame log the one who introduced this code a few days ago.
Flags: needinfo?(masayuki)
Thank you for the quick report. Actually, this is a mistake of bug 1286464.
Assignee: nobody → masayuki
Blocks: 1286464
Status: NEW → ASSIGNED
Component: DOM: Events → Event Handling
Flags: needinfo?(masayuki)
Keywords: regression
OS: Unspecified → All
Hardware: Unspecified → All
Version: unspecified → Trunk
Hmm, I cannot reproduce this bug with creating tests. Could you provide the DOM tree's screenshot when you type ":)"?
Hey, sorry for the delay replying, I can still grab a content tree snapshot if you need it. Just checked it and yeah, I can't reproduce the bug anymore with that build, thanks!
Flags: needinfo?(ealvarez)
(In reply to Emilio Cobos Álvarez [:emilio] from comment #6) > Hey, sorry for the delay replying, I can still grab a content tree snapshot > if you need it. If you can, I'd like to include the case into automated tests if it's possible. Could you attach a screenshot of Inspector (Ctrl+Shift+C)? Only in the editor is enough. # As far as I investigated, I have no idea how to reproduce it intentionally. It might be related to empty text node or something... > Just checked it and yeah, I can't reproduce the bug anymore with that build, > thanks! Thank you! If you cannot attach screenshot soon, I'll request reviews for the patches. Let me know if so.
Ah, bad timing. I got reply from my friend by google hangouts. So, I got the DOM tree now. Although, the DOM tree is not special. It has only a text node created by Gecko, sigh... Anyway, thank you for your report and test! I'll request review.
Heh, was just doing it, ok then, good luck! :)
FYI: I'm still not sure how to reproduce this bug because this must be tested by the automated tests... Anyway, I found bugs of the method and its fix causes setting node offset to 0. Therefore, it needs to check the offset before decrementing it.
Comment on attachment 8781512 [details] Bug 1295354 part.1 ContentEventHandler::GetLastFrameInRangeForTextRect() shouldn't set nextNodeOfRangeEnd to the start node of aRange because the start node should always be in the range https://reviewboard.mozilla.org/r/71920/#review69434
Attachment #8781512 - Flags: review?(bugs) → review+
Comment on attachment 8781513 [details] Bug 1295354 part.2 ContentEventHandler::GetLastFrameInRangeForTextRect() shouldn't use empty text node as far as possible https://reviewboard.mozilla.org/r/71922/#review69446 ::: dom/events/ContentEventHandler.cpp:1584 (Diff revision 1) > if (node == aRange->GetEndParent()) { > nodePosition.mOffset = aRange->EndOffset(); > } else { > nodePosition.mOffset = node->Length(); > } > + // If the text node is empty, we should store current position but If we called nodePosition.mOffset = aRange->EndOffset();, text node might not be empty here. So at least the comment needs to be fixed.
Comment on attachment 8781513 [details] Bug 1295354 part.2 ContentEventHandler::GetLastFrameInRangeForTextRect() shouldn't use empty text node as far as possible https://reviewboard.mozilla.org/r/71922/#review69448
Attachment #8781513 - Flags: review?(bugs) → review+
Comment on attachment 8781513 [details] Bug 1295354 part.2 ContentEventHandler::GetLastFrameInRangeForTextRect() shouldn't use empty text node as far as possible https://reviewboard.mozilla.org/r/71922/#review69446 > If we called nodePosition.mOffset = aRange->EndOffset();, text node might not be empty here. > So at least the comment needs to be fixed. New comment: > // If the text node is empty or the last node of the range but the index > // is 0, we should store current position but continue looking for > // previous node (If there are no nodes before it, we should use current > // node position for returning its frame).
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/31c1f6df66d2 part.1 ContentEventHandler::GetLastFrameInRangeForTextRect() shouldn't set nextNodeOfRangeEnd to the start node of aRange because the start node should always be in the range r=smaug https://hg.mozilla.org/integration/autoland/rev/785aa98cb54c part.2 ContentEventHandler::GetLastFrameInRangeForTextRect() shouldn't use empty text node as far as possible r=smaug
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: