Closed
Bug 181463
Opened 22 years ago
Closed 22 years ago
Clicking on slider with hidden thumb crashes browser [@ nsSliderFrame::HandlePress ]
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: chuckie, Assigned: hyatt)
Details
(Keywords: crash, testcase)
Crash Data
Attachments
(3 files, 2 obsolete files)
555 bytes,
application/vnd.mozilla.xul+xml
|
Details | |
1.50 KB,
patch
|
jrgmorrison
:
review+
jrgmorrison
:
superreview+
|
Details | Diff | Splinter Review |
1011 bytes,
text/plain
|
Details |
User-Agent: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.3a) Gecko/20021122 Build Identifier: Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.3a) Gecko/20021122 Clicking any area of a XUL <slider> whose <thumb>'s CSS "display" property is "none" crashes the browser. I'll include a testcase that has two sliders: one with a visible thumb and the other with a hidden thumb. The hidden thumb slider is the problem. Reproducible: Always Steps to Reproduce: 1. Load the testcase XUL document 2. Click any part of the slider on the right. Actual Results: The browser crashed and the talkback window appeared. Expected Results: The behavior should probably be the same as when the thumb is visible: dragging the (invisible) thumb or clicking above or below it should updates the curpos attribute and move the thumb. Talkback crash ID is TB14253891X. Unfortunately, I can't find a core file on my system. I have a small testcase, which I'll attach shortly. As to the question "why would anyone want a slider with an invisible thumb?!", my little application restyles the slider and thumb to be a moveable fret marker on a guitar fretboard. For open or mute notes, another button in the UI makes the marker disappear and reappear. It really does make sense, trust me. The testcase uses no special styling and I've reproduced the problem with the Modern theme.
Reporter | ||
Comment 1•22 years ago
|
||
Bugzilla's "Create a New Attachment" didn't work, so here's the testcase: <?xml version="1.0"?> <?xml-stylesheet href="chrome://global/skin/" type="text/css"?> <?xml-stylesheet href="chrome://leftychord/skin/" type="text/css"?> <window id="hiddenthumb" title="Hidden Thumb Crash Test" style="height: 375px; width: 250px" xmlns:html="http://www.w3.org/1999/xhtml" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <box height="100"> <slider orient="vertical"> <thumb/> </slider> <slider orient="vertical"> <thumb style="display:none"/> </slider> </box> </window>
Comment 2•22 years ago
|
||
Comment 3•22 years ago
|
||
confirming using build 20021119 on Win2k.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Whiteboard: TB14253891X
nsSliderFrame::HandlePress [c:/builds/seamonkey/mozilla/layout/xul/base/src/nsSliderFrame.cpp, line 1166] nsFrame::HandleEvent [c:/builds/seamonkey/mozilla/layout/html/base/src/nsFrame.cpp, line 979] nsSliderFrame::HandleEvent [c:/builds/seamonkey/mozilla/layout/xul/base/src/nsSliderFrame.cpp, line 733] PresShell::HandleEventInternal [c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6246] PresShell::HandleEvent [c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp, line 6136] nsViewManager::HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 2209] nsView::HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 304] nsViewManager::DispatchEvent [c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp, line 1949] HandleEvent [c:/builds/seamonkey/mozilla/view/src/nsView.cpp, line 83] nsWindow::DispatchEvent [c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1073] nsWindow::DispatchWindowEvent [c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1090] nsWindow::DispatchMouseEvent [c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5283] ChildWindow::DispatchMouseEvent [c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 5538] nsWindow::ProcessMessage [c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 4065] nsWindow::WindowProc [c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp, line 1339] USER32.dll + 0x2a2b8 (0x77e3a2b8) USER32.dll + 0x45b1 (0x77e145b1) USER32.dll + 0xa752 (0x77e1a752) nsAppShellService::Run [c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp, line 472] main1 [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1557] main [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1905] WinMain [c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp, line 1925] WinMainCRTStartup() KERNEL32.dll + 0x2847c (0x77ea847c)
Updated•22 years ago
|
Keywords: stackwanted
Summary: Clicking on slider with hidden thumb crashes browser → Clicking on slider with hidden thumb crashes browser [@ nsSliderFrame::HandlePress ]
Whiteboard: TB14253891X
Comment 5•22 years ago
|
||
match the other null-check spackle fixes that have been applied to that file (and clean up some indentation and line-wrap while I'm there)
Comment 6•22 years ago
|
||
for your viewing pleasure
Comment 7•22 years ago
|
||
Comment on attachment 107195 [details] [diff] [review] null check (diff -u) This also includes some whitespace cleanup. There is a diff -wu version on the bug
Attachment #107195 -
Flags: superreview?(hyatt)
Attachment #107195 -
Flags: review?(varga)
Comment 8•22 years ago
|
||
Comment on attachment 107195 [details] [diff] [review] null check (diff -u) looks good r=varga
Attachment #107195 -
Flags: review?(varga) → review+
Comment 9•22 years ago
|
||
Comment on attachment 107195 [details] [diff] [review] null check (diff -u) > NS_IMETHODIMP > nsSliderFrame::HandlePress(nsIPresContext* aPresContext, >- nsGUIEvent* aEvent, >- nsEventStatus* aEventStatus) >+ nsGUIEvent* aEvent, >+ nsEventStatus* aEventStatus) > { > PRBool isHorizontal = IsHorizontal(); Get rid of this variable (see below). Unless for some odd, uncommented reason, you need to sample it here, before calling mFrames.FirstChild() or thumbFrame->GetRect? >+ >+ nscoord change = 1; >+ if ((isHorizontal && aEvent->point.x < thumbRect.x) || >+ (!isHorizontal && aEvent->point.y < thumbRect.y)) That is to say: + if (IsHorizontal() ? aEvent->point.x < thumbRect.x) : aEvent->point.y < thumbRect.y) Fix that, and sr=brendan (I bet hyatt won't mind my jumping in here). /be
Comment 10•22 years ago
|
||
Oops, extra right paren in my ?: condition -- but you get the idea (where's emacs electric-c-mode in this textarea when i need it? ;-). /be
Comment 11•22 years ago
|
||
No, there's no side effect in calling IsHorizontal(), so lose the temporary and modify the if statement. Thanks.
Attachment #107195 -
Attachment is obsolete: true
Attachment #107196 -
Attachment is obsolete: true
Comment 12•22 years ago
|
||
Comment on attachment 107205 [details] [diff] [review] null-check, use ternary inside if, whitespace cleanup (diff -u) (Hmm, this may wind up looking like r=jrgm,sr=jrgm but it's r=varga,sr=brendan.)
Attachment #107205 -
Flags: superreview+
Attachment #107205 -
Flags: review+
Comment 13•22 years ago
|
||
Comment 14•22 years ago
|
||
checked in
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Comment 15•22 years ago
|
||
I do not crash anymore using build 2002112305 on Linux.
Reporter | ||
Comment 16•22 years ago
|
||
Works for me as well. Thanks for the speedy fix!
Comment 17•22 years ago
|
||
Comment on attachment 107195 [details] [diff] [review] null check (diff -u) Hmm, the patch manager doesn't automagically remove review requests from obsolete bugs (so do it manually)
Attachment #107195 -
Flags: superreview?(hyatt) → superreview?
Comment 18•22 years ago
|
||
Comment on attachment 107195 [details] [diff] [review] null check (diff -u) Sigh.
Attachment #107195 -
Flags: superreview?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: shrir → xptoolkit.widgets
Updated•13 years ago
|
Crash Signature: [@ nsSliderFrame::HandlePress ]
You need to log in
before you can comment on or make changes to this bug.
Description
•