1434340 - Crash in nsFrame::HandlePress

Status: RESOLVED DUPLICATE of bug 1434273

(Core :: Layout, defect)

Description

[Julien Wajsberg [:julienw]]

This bug was filed from the Socorro interface and is
report bp-e467adf6-0f11-4795-bf3f-1ce850180130.
=============================================================

Top 10 frames of crashing thread:

0  @0xfffffffffffffff8 
1 libxul.so nsFrame::HandlePress layout/generic/nsFrame.cpp:4167
2 libxul.so nsFrame::HandleEvent [clone .cold.932] 
3 libxul.so nsImageFrame::HandleEvent layout/generic/nsImageFrame.cpp:2130
4 libxul.so mozilla::EventTargetChainItem::HandleEventTargetChain 
5 libxul.so mozilla::EventDispatcher::Dispatch 
6 libxul.so mozilla::PresShell::DispatchEventToDOM 
7 libxul.so mozilla::PresShell::HandleEventInternal 
8 libxul.so mozilla::PresShell::HandleEvent 
9 libxul.so nsViewManager::DispatchEvent 

=============================================================

Comment 1

[Julien Wajsberg [:julienw]]

This happened to me in google docs, adding a comment. This happens 100% of the time, I think.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I can repro, thanks, I can try to take a look.

Comment 4

[Marcia Knous [:marcia]]

The duplicate bug has the regression range:

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=474d58c9137360c0fa1c85cdd11e3313b33b7cad&tochange=9746e0a0a81cc089ff65e30ae902864846cd1b94

Bug 1433846 touched code in this area.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

This is a regression from bug 1432977.

Comment 6

[Marcia Knous [:marcia]]

Adding Windows signature.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Actually this is a long time issue, and can be s-s I think.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Generated content for images shouldn't claim to be HTML elements.

Alternative fix is making them inherit from nsGenericHTMLElement instead of nsXMLElement.

The cause of the regression is this diff:

diff --git a/accessible/generic/Accessible.cpp b/accessible/generic/Accessible.cpp
index 4188eb2b5848..2a021343000b 100644
--- a/accessible/generic/Accessible.cpp
+++ b/accessible/generic/Accessible.cpp
@@ -13,6 +13,7 @@
 #include "nsAccUtils.h"
 #include "nsAccessibilityService.h"
 #include "ApplicationAccessible.h"
+#include "nsGenericHTMLElement.h"
 #include "NotificationController.h"
 #include "nsEventShell.h"
 #include "nsTextEquivUtils.h"
@@ -1072,11 +1073,8 @@ Accessible::NativeAttributes()
   nsAccUtils::SetAccAttr(attributes, nsGkAtoms::tag, tagName);
 
   // Expose draggable object attribute.
-  nsCOMPtr<nsIDOMHTMLElement> htmlElement = do_QueryInterface(mContent);
-  if (htmlElement) {
-    bool draggable = false;
-    htmlElement->GetDraggable(&draggable);
-    if (draggable) {
+  if (auto htmlElement = nsGenericHTMLElement::FromContent(mContent)) {
+    if (htmlElement->Draggable()) {
       nsAccUtils::SetAccAttr(attributes, nsGkAtoms::draggable,
                              NS_LITERAL_STRING("true"));
     }

Of course the bug was pre-existing.

Boris, this is a long-time-ago type confusion, I'm not sure I can assess whether it's too bad, or just bad.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Testcase

Just (try to) drag the image around.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8946721 [details] [diff] [review]
Generated content for images shouldn't claim to be HTML elements.

Err, of course this won't work as is because we rely on this to find the right FCData. 1sec.

Comment 12

[Julien Cristau [:jcristau] (back April 22)]

clearing tracking flags in favour of bug 1434273