Closed Bug 78344 Opened 23 years ago Closed 23 years ago

event.clientX/Y should not be converted to screenX/Y when target is in popup. autocomplete scrollbar doesn't allow dragging.

Categories

(Core :: XUL, defect, P4)

x86
All
defect

Tracking

()

VERIFIED FIXED
mozilla0.9.6

People

(Reporter: hwaara, Assigned: hyatt)

References

Details

(Keywords: regression, Whiteboard: [br])

Attachments

(2 files)

If I type something in the URLbar, and then click the little arrow to get
autocomplete suggestions - I can't scroll. The scrollbar simply doesn't
function. I can't drag it.
Build ID: 2001043004

For me, it drags but only after moving much farther than normal.  It looks like
the mouse pointer is offset by about one scrollbar's length from the normal
position.
[ ] - Scrollbar
 |  - Empty trough
 |  - where you need to point to get scrollbar to move to location marked empty
trough.  

This is in moderm theme if that makes a difference.
I'm having no problems with this. Have tried it with both Classic and Modern
themes, on build linux 2001043021.
hwaara, bugs rarely get fixed in Browser General and never when they're assigned
to me.  You might do better assigning this to hewitt (he wrote the new
autocomplete) or someone in XPApps.  Also, please read the bug writing
guidelines at http://www.mozilla.org/quality/bug-writing-guidelines.html to see
the kind of information we need in a bug report.  Of particular interest and
lacking in your report is the build ID. If this is a build from yesterday
morning then it has the old autocomplete.  If it is a build from yesterday
afternoon, evening or this morning then it has the new autocomplete.  Without
that critical piece of information this bug is that much harder to assign.
moving out of browser general.
Assignee: asa → alecf
Component: Browser-General → URL Bar
QA Contact: doronr → claudius
Sorry, I was in a hurry when I wrote the report. More information:

Build id is today's date, since I pulled this morning. I believe it's the new
autocomplete since (I think) it worked fine yesterday.

Related to the other bug I filed, bug 78343?
The real problem here goes beyond autocomplete.  The problem is that, somewhere
along the way, the clientX and clientY properties are being converted to
screenX/screenY.  I reckon clientY is used to determine where the mouse is
relative to the scrollbar thumb.  The client properties are expected to be in
client coordinates, and are compared to the client coordinates of the thumb.

Not sure if this belongs to DOM or XPToolkit, but I'll start with XPToolkit.

If you want more proof of my theory, notice that the lower your window is, the
further south you have to move your mouse before the thumb begins to drag.
Assignee: alecf → trudelle
Component: URL Bar → XP Toolkit/Widgets
QA Contact: claudius → jrgm
Summary: scrollbar in autocomplete in URLbar can't scroll → event.clientX/Y should not be converted to screenX/Y when target is in popup
Assignee: trudelle → pinkerton
Thanks for the clarification Joe. I can verify that the amount of dragging
required before the thumb moves is greater at larger Y coords, to the point that
you don't have enough real estate to drag it at all near the bottom.  I don't
think this qualifies as "Major" severity though, since there really isn't any
loss of function, at least in the current example, just partial loss of access
to that function.  ->pinkerton
i've seen this too.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Target Milestone: mozilla0.9.1 → mozilla0.9.2
Isn't this sort of a showstopper for the whole autocomplete widget feature? I
mean, we can't have this broken in 0.9.1 (re: 0.9.2 TM).
this just affects the scroll thumb. everything else, including scroll arrows, 
works. so it's annoying, but not a _showstopper_
*** Bug 78925 has been marked as a duplicate of this bug. ***
Whiteboard: [br]
Sorry for the spam, just leaving my mark.
*** Bug 82575 has been marked as a duplicate of this bug. ***
*** Bug 81911 has been marked as a duplicate of this bug. ***
Here's a guess at where this is happening, based mostly on the comment.

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsSliderFrame.cpp#45
9
"everything else" doesn't quite work.. Wheelscrolling has problems.
Are bug 83009 and bug 82429 dups of this?
Doesn't fit the profile for 0.9.2, but a fix could still be considered during
the 'limbo' period.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
*** Bug 83771 has been marked as a duplicate of this bug. ***
OS: All. I see this on linux as well.
OS: Windows 98 → All
Blocks: 82429
*** Bug 85031 has been marked as a duplicate of this bug. ***
> Doesn't fit the profile for 0.9.2

And that would be what? Whilst the scrollbar is somewhat usable, it's still a
usability problem, one that is highly annoying. Releasing nscp6.5 with this bug
would be an interesting choice of priorities, imvho.
In a GUI based application a scrollbar is part of the basic infrastructure that
just should plain work. Any malfunction invites ridicule - and rightfully so.
This has to be fixed. Users believe it doesn't even work. And are partly right:

When the top of a browser or mail window is placed halfway the screen, the
"trigger point" of a scroll is way beyond reach: One can't drag the mouse far
enough down to even start a drag.
RKA: you're right. If I make the top of my browser window align with the middle
of the page vertically, and open the URLBar history, I cannot scroll at all. But
I can click on the empty area above/below the bar to move the bar into that
place. However I've known a lot of people who do not realise such scrolling
functionality exists.

Nominating for nsbeta1 (assuming there will be a beta before next release) and
nscatfood.
Keywords: nsbeta1, nsCatFood
How do you tell if the target is in a popup?
this is so minor of a glitch, i'm amazed we're arguing about it given all the
other stuff that crashes or doesn't work. the user can still scroll with the
arrows. no functionality is lost.
<rant>
Sorry mate, calling some of the most basic and simple functionality that even 
the unexperienced of all users would find disturbing, shows some detachment from 
reality.

Better leave out a mail and news reader altogether than declaring basic bugs 
minor glitches. After all, you don't want to do a technology showcase that 
internally features the most elegant class hierarchies and interfaces and 
standards, but is not usable by the stupid user.

I think, there is a lesson to be learnt here.
</rant>
*** Bug 85863 has been marked as a duplicate of this bug. ***
*** Bug 85927 has been marked as a duplicate of this bug. ***
*** Bug 86015 has been marked as a duplicate of this bug. ***
modifying the summary to something people might search for to prevent dupes.
Summary: event.clientX/Y should not be converted to screenX/Y when target is in popup → event.clientX/Y should not be converted to screenX/Y when target is in popup. autocomplete scrollbar doesn't allow dragging.
*** Bug 86806 has been marked as a duplicate of this bug. ***
*** Bug 87846 has been marked as a duplicate of this bug. ***
Adding mostfreq at 10 dups.
Keywords: mostfreq
*** Bug 88572 has been marked as a duplicate of this bug. ***
*** Bug 88659 has been marked as a duplicate of this bug. ***
*** Bug 90285 has been marked as a duplicate of this bug. ***
*** Bug 91258 has been marked as a duplicate of this bug. ***
Doesn't look like this is getting fixed before the freeze tomorrow night.
Pushing out a milestone.  Please correct if I'm mistaken.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
*** Bug 93785 has been marked as a duplicate of this bug. ***
*** Bug 95519 has been marked as a duplicate of this bug. ***
*** Bug 95974 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.4 → mozilla0.9.5
*** Bug 97359 has been marked as a duplicate of this bug. ***
Blocks: advocacybugs
Well, I can make this _less_ screwy.  I did some digging in 
nsDomEvent::GetClientY and can't get things to work perfectly.  I can, however, 
get it the thumb to consistently jump down a bunch when you start dragging it.  
The distance of the jump is the same regardless of where the window is on-
screen.  Hopefully I'm missing something obvious and someone else can figure it 
out from here.

Note in the diff that this is only for GetClientY.  Whatever we find that works 
should be copied for GetClientX.  The NS_RELEASE/break that's commented out 
seemed to have the same effect.  This doesn't seem to have any effect on 
regular scrollbars or scrollbars on form objects.
This seems to screw up the scrollbar in menulists in the classic theme (eg. 
Prefs > Message Display > Character Coding), but they are already screwed up in 
a different way.  Testing of any patch should include this menulist as well.
*** Bug 98412 has been marked as a duplicate of this bug. ***
Blocks: 99194
Is this a stop ship for nsbranch? Should we nsbranch+ it?
*** Bug 99745 has been marked as a duplicate of this bug. ***
*** Bug 99952 has been marked as a duplicate of this bug. ***
i'm not going to get to this any time soon.
Assignee: pinkerton → trudelle
Status: ASSIGNED → NEW
Target Milestone: mozilla0.9.5 → ---
->hyatt, p4/096/nsbranch-  I appreciate everyone's concerns, and regret having
such a fundamental defect, but this is how we have to prioritize in order to
ever ship anything.  If you feel so strongly about this,  attach a patch, or
convince someone else to do so. 
Assignee: trudelle → hyatt
Keywords: nsbranchnsbranch-
Priority: -- → P4
Target Milestone: --- → mozilla0.9.6
Sorry, but you still don't understand how the browser war works. No user cares 
about xyz technology being used under the hood. This is just for the developers 
pleasure. Instead, they try a simple standard feature like the URLbar and find 
it doesn't work. Result: "This browser is ****. After so many years of 
development they can't even get a simple widget right." And back to IE.

Would you buy a car with an F1 engine if you can't open the doors?
*** Bug 101453 has been marked as a duplicate of this bug. ***
*** Bug 102859 has been marked as a duplicate of this bug. ***
dean was on the right track... just missed a similar spot in the slider code.
Status: NEW → ASSIGNED
r=blake
Attachment #52421 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
/me high-fives hyatt -- Thanks for picking this up!

Question, though.  Does your patch leak?  In mine, I added an extra NS_RELEASE
because there's an intentional NS_ADDREF before the loop ("//Add extra ref since
loop will free one.")
I'll verify this, but I'm waiting to hear from Hyatt about my leak question.
Verifying.  This doesn't look like it leaks.  The additional NS_ADDREF occurs in
nsWidget::GetParent(), which isn't called if the break is hit.
Status: RESOLVED → VERIFIED
Keywords: mozilla1.0, oeone
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: