Zimbra emails don't open on nightly build

RESOLVED FIXED

Status

()

Core
DOM: Events
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: Boriss, Assigned: Neil Deakin)

Tracking

(Depends on: 1 bug, {dogfood, regression})

Trunk
dogfood, regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
Zimbra loads in the nightly build, but clicking on an email to open it does not give a response other than the cursor changing from a hand to an arrow.
(Reporter)

Comment 1

9 years ago
In Error Console: 

Warning: The 'charCode' property of a keyup event should not be used. The value is meaningless.
Source file: https://mail.mozilla.com
(Reporter)

Comment 2

9 years ago
Build identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a1pre) Gecko/20090914 Minefield/3.7a1pre
Not a JS bug in any case.

/be
Assignee: general → nobody
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Component: JavaScript Engine → DOM: Events
QA Contact: general → events
Resolution: --- → DUPLICATE
Duplicate of bug: 467593
Sorry, not a dup. What's going wrong?

/be
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Comment 5

9 years ago
Comment 1 shows just a valid warning. That shouldn't cause any problems.

What is the regression range for this?

Updated

9 years ago
Keywords: regressionwindow-wanted

Comment 6

9 years ago
2009-09-13 works, -14 doesn't

Comment 7

9 years ago
Somehow caused by bug 503943.
Blocks: 503943

Updated

9 years ago
Keywords: regressionwindow-wanted

Comment 8

9 years ago
Zimbra does use element.setCapture()/.releaseCapture() on body element.
Do we not dispatch click or what?
(Assignee)

Comment 9

9 years ago
The click event is not firing because the mouseup is occurring on a different element (the body) instead of whatever was clicked on.

Now normally, even when mouse capturing is used, events should still reach descendants of the element doing the capturing. However, in this particular case, almost all content is absolutely positioned, so the frames are outside the body.

One possible solution is to just treat a capture on a <body> tag as a capture on the root element instead.
The question is: how does IE handle this case?
(Assignee)

Comment 11

9 years ago
I can't really test it for a while, but presumably it doesn't consider its positioned elements to be outside of the body. Or, its different event model might cause it to fire on the right element anyway.
(Assignee)

Updated

9 years ago
Duplicate of this bug: 516722
(Assignee)

Comment 13

9 years ago
Created attachment 400775 [details] [diff] [review]
testing this right now

if we want to take this route, here is a patch that does it
Assignee: nobody → enndeakin
Status: REOPENED → ASSIGNED
(Assignee)

Comment 14

9 years ago
So IE just treats:

<body>
  <p style="position:absolute">...</p>
</body>

as the <p> being inside the body. Whereas Mozilla's frame tree has <p> outside the body.

Maybe an alternative is some modification to GetFrameForPoint.
setCapture works on DOM, not on layout object tree, right?
Presshell could perhaps traverse DOM and not use frame tree - 
at least if that is what IE does.
From MSDN
"When the bContainerCapture parameter is set to true, a container object, such as a  div, captures mouse events for all objects in it. By passing the value false, objects in that container can fire events, and cancel event bubbling."
I interpret that so that setCapture works on DOM.

Needs testing, ofc.
(Assignee)

Comment 17

9 years ago
I'm not clear what you're asking here. setCapture causes a pointer to an nsIContent to be stored indicating which element is capturing the mouse. When a mouse event occurs, the primary frame for it is retrieved and uses the normal mouse hit testing which uses frames and display lists to find a child within that frame to target at.
I mean something like...
If there is capturing content, presshell starts currently from the primary frame of the capturing content and then tries to find the target frame inside that.
But I think it should work other way when !gCaptureInfo.mRetargetToElement.
First use the normal hit testing, then if the target frame's content is a descendant of the capturing content, normal event handling can be used.
Does this make sense?

Btw, is it possible (and on purpose) that when !gCaptureInfo.mRetargetToElement,
targetFrame can be in some subshell and the event is dispatched there?
(In reply to comment #18)
> But I think it should work other way when !gCaptureInfo.mRetargetToElement.
> First use the normal hit testing, then if the target frame's content is a
> descendant of the capturing content, normal event handling can be used.
> Does this make sense?

That makes sense to me.
This really sucks for me. Can we get this fixed or backed out or something?
Keywords: dogfood
(Assignee)

Comment 21

9 years ago
Created attachment 401566 [details] [diff] [review]
implement as discussed

The new test fails on Windows, but I can't test there currently.
Attachment #400775 - Attachment is obsolete: true
(Assignee)

Comment 22

9 years ago
Created attachment 402119 [details] [diff] [review]
fix test by removing selection first
Attachment #401566 - Attachment is obsolete: true
Attachment #402119 - Flags: superreview?(Olli.Pettay)
Attachment #402119 - Flags: review?(roc)
So (In reply to comment #22)
> Created an attachment (id=402119) [details]
So is this the previous patch + selection removal?
(Assignee)

Comment 24

9 years ago
Sorry, the comment refers to adding a line to clear the selection in the test, a change from the previous patch.
Comment on attachment 402119 [details] [diff] [review]
fix test by removing selection first

>+    // if a node is capturing the mouse, check if the event needs to be
>+    // retargeted at the capturing content instead. This will be the case when
>+    // capture retargeting is being used, no frame was found or the frame's
>+    // content is not a descendant of the capturing content.
>+    if (capturingContent &&
>+        (gCaptureInfo.mRetargetToElement ||
>+         !targetFrame || !targetFrame->GetContent() ||
>+         !nsContentUtils::ContentIsCrossDocDescendantOf(targetFrame->GetContent(),
>+                                                       capturingContent))) {
Nit, add a space before capturingContent.

Does IE also allow sub documents to capture the event when not retargeting to the
element? If it does the same what this patch, then sr=me.

Or if it doesn't do that, then you'll probably need to use 
ContentIsDescendantOf(). if that works the same way as IE, sr=me
Attachment #402119 - Flags: superreview?(Olli.Pettay) → superreview+
(Assignee)

Comment 26

9 years ago
http://hg.mozilla.org/mozilla-central/rev/0580094398f3
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
(Assignee)

Updated

9 years ago
Flags: in-testsuite+

Updated

9 years ago
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Updated

9 years ago
Blocks: 519693

Updated

7 years ago
Depends on: 668640

Updated

2 years ago
Depends on: 623432
You need to log in before you can comment on or make changes to this bug.