Events should go to capturing views even when clip rect is empty [Patch included]

RESOLVED FIXED

Status

()

Core
Layout: View Rendering
P1
normal
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Stef Walter, Assigned: roc)

Tracking

({testcase})

Trunk
testcase
Points:
---
Bug Flags:
blocking1.5b +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

15 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.4b) Gecko/20030516 Mozilla Firebird/0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5b) Gecko/20030716

Tree type splitters don't work inside of a scrolled view. After researching the
problem, I found the issues to be in nsViewManager where events aren't in some
cases delivered to views that have an empty clip rect, even when that view is
capturing.

Included is a patch. I need this fixed before I can complete bug #212789

Reproducible: Always

Steps to Reproduce:
1. Open XUL attachment
2 [review]. Note how tree splitters on top bar work fine, but splitters 
   inside the scrolled view cause a mess.


Actual Results:  
Splitters in top set of tree columns work, bottom set doesn't.

Expected Results:  
Behavior should be identical in the two cases, as all that's present is an
additional view (overflow: -moz-scrollbars-borizontal)
(Reporter)

Updated

15 years ago
Blocks: 212789
(Reporter)

Comment 1

15 years ago
Created attachment 128028 [details]
Test case
(Reporter)

Comment 2

15 years ago
Created attachment 128030 [details] [diff] [review]
Patch to fix the issue
Nate, you should request r= and sr= on the patch, maybe from roc and dbaron? 
(I'm not quite sure who the best candidates are, asking around #mozilla may 
help.)
Assignee: roc+moz → nielsen
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: testcase
Summary: Events should go to views even when clip rect is empty [Patch included] → [EVENTTARG] Events should go to views even when clip rect is empty [Patch included]
Summary: [EVENTTARG] Events should go to views even when clip rect is empty [Patch included] → Events should go to capturing views even when clip rect is empty [Patch included]
(Reporter)

Updated

15 years ago
Attachment #128030 - Flags: review?(roc+moz)
Could you attach a diff -pu12 (unified diff, more context, show function name)?
 I can't tell what function you're modifying.
(Reporter)

Comment 5

15 years ago
Created attachment 128039 [details] [diff] [review]
The same patch (with diff -pu12)
Attachment #128030 - Attachment is obsolete: true
Perhaps it would be better to pass the |aEventProcessing| argument through to
AddToDisplayList and make this change conditional on that?

Really, though, it seems like the right way to fix this would really be to make
nsViewManager::HandleEvent only use BuildEventTargetList when it receives a
mouse event for which aCaptured is false.  But that would surely break something...
(Reporter)

Comment 7

15 years ago
Your call. I'm unfamiliar with the view manager code. What I don't understand is
why this only occurs when inside another view (the scrolling one). 
(Reporter)

Comment 8

15 years ago
Created attachment 128166 [details] [diff] [review]
Patch incorporating dbaron's suggestion
(Reporter)

Updated

15 years ago
Attachment #128039 - Attachment is obsolete: true
A simpler patch would be to just remove the empty-clip-rect check:

-  if (empty) {
-    return PR_FALSE;

The check is not really needed since normally an empty cliprect will mean an
empty intersection is detected below.

Note that aAssumeIntersection should get set for the capturing view so your
events should be received. Try it and see.
(Reporter)

Comment 10

15 years ago
Yes this is what the original patch did. attachment #212789 [details] [diff] [review]
(Reporter)

Comment 11

15 years ago
Created attachment 128246 [details] [diff] [review]
Simple patch as per roc's comment (#9)

Bad attachment id above. Here's the simple patch again to avoid confusion.
Comment on attachment 128246 [details] [diff] [review]
Simple patch as per roc's comment (#9)

oh great! r+sr=roc+moz

but don't bother commenting all this out. Just remove the code. We don't need
gravestones for dead bugs.

Nice catch BTW. good work!
Attachment #128246 - Flags: superreview+
Attachment #128246 - Flags: review+
(Reporter)

Comment 13

15 years ago
Over to roc for check-in.
Assignee: nielsen → roc+moz
Priority: -- → P1
Comment on attachment 128246 [details] [diff] [review]
Simple patch as per roc's comment (#9)

simple fix for a simple bug
Attachment #128246 - Flags: approval1.5b?

Updated

15 years ago
Flags: blocking1.5b+

Updated

15 years ago
Attachment #128246 - Flags: approval1.5b? → approval1.5b+
Checked in. Thanks Nate!!!
actually marking FIXED
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.