Summary: | [Qt] fast/text/find-hidden-text.html | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||||
Component: | Platform | Assignee: | Benjamin Poulain <benjamin> | ||||||||
Status: | CLOSED FIXED | ||||||||||
Severity: | Major | CC: | benjamin, commit-queue, hausmann, jwieczorek, kent.hansen | ||||||||
Priority: | P1 | Keywords: | Qt, QtTriaged | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 35784 | ||||||||||
Attachments: |
|
Description
Robert Hogan
2009-12-24 12:05:40 PST
Created attachment 46207 [details]
A reduction
Reproducable when selecting the text in the following example.
Please follow the QtWebKit bug reporting guidelines when reporting bugs. See http://trac.webkit.org/wiki/QtWebKitBugs Specifically: - The 'QtWebKit' component should only be used for bugs/features in the public QtWebKit API layer, not to signify that the bug is specific to the Qt port of WebKit http://trac.webkit.org/wiki/QtWebKitBugs#Component - Add the keyword 'Qt' to signal that it's a Qt-related bug http://trac.webkit.org/wiki/QtWebKitBugs#Keywords I'm not able to reproduce this on Mac with r55986, Qt 4.7. I don't have the problem with the reduction on 4.7. Is there anything special to do in order to reproduce the crash? Do you select all the text or a specific sub-string? (In reply to comment #4) > I don't have the problem with the reduction on 4.7. I can still reproduce this with Qt 4.7 and WebKit trunk. My suspicion is that Kent and you are using a style that does not call the standard QCommonStyle's ::subControlRect() implementation in this case and at the same time does not trigger this crash. > Is there anything special to do in order to reproduce the crash? Do you select all the text or a specific sub-string? It's crashing right after I start the selection. (In reply to comment #5) > My suspicion is that Kent and you are using a style that does not call > the standard QCommonStyle's ::subControlRect() implementation in this case and at the same time does not trigger this crash. Which style are you using? (In reply to comment #6) > (In reply to comment #5) > > My suspicion is that Kent and you are using a style that does not call > > the standard QCommonStyle's ::subControlRect() implementation in this case and at the same time does not trigger this crash. > > Which style are you using? I can reproduce this with QGtkStyle as well as pretty much any standard style included in Qt. I recall reproducing it with Oxygen in the past but I can't say for sure. (In reply to comment #7) > I can reproduce this with QGtkStyle as well as pretty much any standard style included in Qt. I recall reproducing it with Oxygen in the past but I can't say for sure. OK, I checked Oxygen and it's not crashing. I am using Oxygen. Confirmed with plastique style: 0x00007ffff37e7461 in QCommonStyle::subControlRect (this=0x76ee60, cc=QStyle::CC_ScrollBar, opt=0x7ffff7dddae0, sc=QStyle::SC_ScrollBarGroove, widget=0x0) at /home/ikipou/dev/oslo-staging-1/src/gui/styles/qcommonstyle.cpp:3989 3989 sliderlen = (qint64(scrollbar->pageStep) * maxlen) / (range + scrollbar->pageStep); (gdb) print range $1 = 16 (gdb) print scrollbar->pageStep $2 = -16 Forgot to mention the merge request here: http://qt.gitorious.org/qt/qt/merge_requests/2387 and also see Benjamin's comment there: "“Revise and resubmit” because this misses an autotest. I also don’t agree with the fix. since (scrollbar->maximum != scrollbar->minimum), range is Superior to 0. The way to have 0 for the denominator is to have scrollbar->pageStep equals -range. For me it looks like a wrong value to have QStyleOptionSlider with a negative page step." Created attachment 55567 [details]
Patch
Robert, If I understand correctly, there is already a test to reproduce the crash?
(In reply to comment #11) > Created an attachment (id=55567) [details] > Patch > > Robert, If I understand correctly, there is already a test to reproduce the crash? Yup: fast/text/find-hidden-text.html ;-) That patch looks like a good spot! Created attachment 55570 [details] Patch with test > Yup: fast/text/find-hidden-text.html ;-) Arg, of course, it was carefully hidden in plain sight in the title :) Same patch, but remove the test from the skipped list. (In reply to comment #13) > Created an attachment (id=55570) [details] > Patch with test > > > Yup: fast/text/find-hidden-text.html ;-) > > Arg, of course, it was carefully hidden in plain sight in the title :) > Just like the fix. Shortly before your patch I looked up pageStep in WebCore and didn't blink when I saw opt.pageStep = scrollbar->visibleSize. Odd that so much of the scrollbars tests passed with that in there. Comment on attachment 55570 [details] Patch with test Clearing flags on attachment: 55570 Committed r59151: <http://trac.webkit.org/changeset/59151> All reviewed patches have been landed. Closing bug. Revision r59151 cherry-picked into qtwebkit-2.0 with commit 4aa4ea037fc467194c16a4959caca96a8da4f412 |