Closed Bug 258006 Opened 20 years ago Closed 18 years ago

Mouse-wheel scroll events does not propagate to viewport from fixed pos. elements

Categories

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

defect
Not set
minor

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: MatsPalmgren_bugz, Assigned: masayuki)

References

Details

(Keywords: regression, testcase)

Attachments

(4 files, 2 obsolete files)

Mouse-wheel scroll events does not propagate to viewport from fixed pos.
elements.  (This is spawned off from bug 256538).

The reason is that the fixed pos. frame hangs of the "Fixed-list" of the
Viewport frame which means traversing the frame ancestor chain in
nsEventStateManager::DoScrollText "misses" the ScrollPortFrame(html).

STEPS TO REPRODUCE:
1. Load testcase
2. position mouse over the green area
3. mouse-wheel scroll

ACTUAL RESULTS:
Nothing happens.

EXPECTED RESULTS:
Scrolling occurs.
Attached file Testcase
Keywords: testcase
It works for me on Linux firefox 1.0.2.
It works for me on Linux firefox 1.0.2.
I'm currently using Firefox 1.0.3 on Win98se.

The scrolling works fine. It also works as expected in NS 6.2 and NS 7.2.
(In reply to comment #4)
> I'm currently using Firefox 1.0.3 on Win98se.
> 
> The scrolling works fine. It also works as expected in NS 6.2 and NS 7.2.
> 

Are you sure? It's still definitely broken for me.
1.7 -> OK
1.8 -> not scrolling

This is a regression.
Flags: blocking1.8rc1?
it's too late in the release cycle to block on this bug which is over a year old wa s present in 1.0.x.
Flags: blocking1.8rc1? → blocking1.8rc1-
I guess this should have the keyword, though.
Keywords: regression
*** Bug 279964 has been marked as a duplicate of this bug. ***
This still affects FF 1.5 - build 20051111 - Windows XP SP2.
I still experience this problem as well. 

Using latest seamonkey nightly. 

Attached patch Patch rv1.0 (obsolete) — Splinter Review
On current code, the most ancestor frame of inner fixed positioned frame is not document root frame. Therefore, in this case, the scrollable frame is not found on current document if the fixed positioned context doesn't have scrollable view.

Therefore, my patch processes like as the parent frame of root frame of fixed positioned view is document root frame.
Assignee: mats.palmgren → masayuki
Status: NEW → ASSIGNED
Attachment #209211 - Flags: superreview?(roc)
Attachment #209211 - Flags: review?(roc)
Attached patch Patch rv1.1 (obsolete) — Splinter Review
Attachment #209211 - Attachment is obsolete: true
Attachment #209254 - Flags: superreview?(roc)
Attachment #209254 - Flags: review?(roc)
Attachment #209211 - Flags: superreview?(roc)
Attachment #209211 - Flags: review?(roc)
Attachment #209254 - Flags: branch-1.8.1?(roc)
This approach is a bit complex and hard to understand. I suggest an approach that results in cleaner code:

Define a new helper static function called GetParentFrameToScroll(nsIFrame* aFrame) that returns aFrame's parent, unless aFrame is fixed-pos in which case it returns the root scrollable frame if one exists. Call this method in the 'for' loop instead of using "scrollFrame = scrollFrame->GetParent()".

In GetParentFrameToScroll, detect that aFrame is fixed-pos by checking whether it's in its parent's fixed-pos child list. Not the fastest, but that doesn't matter here. If it is fixed-pos, get the root scroll frame by calling GetRootScrollFrame, which you should move from here
http://lxr.mozilla.org/seamonkey/source/layout/base/nsPresShell.cpp#2662
into a method in nsPresContext that is not virtual and takes no parameters (get the root frame from the frame manager, like here
http://lxr.mozilla.org/seamonkey/source/layout/base/nsPresShell.cpp#4541
Roc:

> In GetParentFrameToScroll, detect that aFrame is fixed-pos by checking whether
> it's in its parent's fixed-pos child list.

What is "fixed-pos child list"?
the list you get when you call parentFrame->GetFirstChild(nsLayoutAtoms::fixedList).
Attached patch Patch rv2.0Splinter Review
Oh, your comment is too late for this patch...
But this patch can fix this bug.
Should we use the list?

# This patch changes pseudo-interface. So, we cannot take this for 1.8 branch...
Attachment #209254 - Attachment is obsolete: true
Attachment #210372 - Flags: superreview?(roc)
Attachment #210372 - Flags: review?(roc)
Attachment #209254 - Flags: superreview?(roc)
Attachment #209254 - Flags: review?(roc)
Attachment #209254 - Flags: branch-1.8.1?(roc)
Comment on attachment 210372 [details] [diff] [review]
Patch rv2.0

That'll do. That looks great.
Attachment #210372 - Flags: superreview?(roc)
Attachment #210372 - Flags: superreview+
Attachment #210372 - Flags: review?(roc)
Attachment #210372 - Flags: review+
checked-in to trunk.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
Did you request branch approval before? I have a request but it looks like you cancelled it. I would approve this for 1.8.1 branch if requested.
Roc:

The latest patch is changing nsIPresShell pseudo-interface, so the patch cannot go to 1.8 branch. If we want to take it to 1.8 branch, we need another patch...
Ah, right. I don't think it's worth it then.
*** Bug 358044 has been marked as a duplicate of this bug. ***
*** Bug 358945 has been marked as a duplicate of this bug. ***
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.