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

VERIFIED FIXED in mozilla0.9.6

Status

()

Core
XUL
P4
major
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Håkan Waara, Assigned: David Hyatt)

Tracking

({regression})

Trunk
mozilla0.9.6
x86
All
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [br])

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
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.

Comment 1

17 years ago
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.

Comment 2

17 years ago
I'm having no problems with this. Have tried it with both Classic and Modern
themes, on build linux 2001043021.

Comment 3

17 years ago
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.

Comment 4

17 years ago
moving out of browser general.
Assignee: asa → alecf
Component: Browser-General → URL Bar
QA Contact: doronr → claudius
(Reporter)

Comment 5

17 years ago
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?

Comment 6

17 years ago
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

Updated

17 years ago
Assignee: trudelle → pinkerton

Comment 7

17 years ago
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

Updated

17 years ago
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Reporter)

Comment 9

17 years ago
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_

Comment 11

17 years ago
*** Bug 78925 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Whiteboard: [br]

Comment 12

17 years ago
Sorry for the spam, just leaving my mark.

Comment 13

17 years ago
*** Bug 82575 has been marked as a duplicate of this bug. ***

Comment 14

17 years ago
*** Bug 81911 has been marked as a duplicate of this bug. ***

Comment 15

17 years ago
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

Comment 16

17 years ago
"everything else" doesn't quite work.. Wheelscrolling has problems.

Comment 17

17 years ago
Are bug 83009 and bug 82429 dups of this?

Comment 18

17 years ago
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

Comment 19

17 years ago
*** Bug 83771 has been marked as a duplicate of this bug. ***

Comment 20

17 years ago
OS: All. I see this on linux as well.
OS: Windows 98 → All

Updated

17 years ago
Blocks: 82429

Comment 21

17 years ago
*** Bug 85031 has been marked as a duplicate of this bug. ***

Comment 22

17 years ago
> 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.
Keywords: mozilla0.9.2, regression, rtm

Comment 23

17 years ago
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.

Comment 24

17 years ago
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

Comment 25

17 years ago
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.

Comment 27

17 years ago
<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>

Comment 28

17 years ago
*** Bug 85863 has been marked as a duplicate of this bug. ***

Comment 29

17 years ago
*** Bug 85927 has been marked as a duplicate of this bug. ***

Comment 30

17 years ago
*** Bug 86015 has been marked as a duplicate of this bug. ***

Comment 31

17 years ago
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.

Comment 32

17 years ago
*** Bug 86806 has been marked as a duplicate of this bug. ***

Comment 33

17 years ago
*** Bug 87846 has been marked as a duplicate of this bug. ***

Comment 34

17 years ago
Adding mostfreq at 10 dups.
Keywords: mostfreq

Comment 35

17 years ago
*** Bug 88572 has been marked as a duplicate of this bug. ***

Comment 36

17 years ago
*** Bug 88659 has been marked as a duplicate of this bug. ***

Comment 37

17 years ago
*** Bug 90285 has been marked as a duplicate of this bug. ***

Comment 38

17 years ago
*** Bug 91258 has been marked as a duplicate of this bug. ***

Comment 39

17 years ago
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

Updated

17 years ago
Keywords: mozilla0.9.2, nsbeta1, nsrtm → mozilla0.9.4, nsBranch

Comment 40

17 years ago
*** Bug 93785 has been marked as a duplicate of this bug. ***

Comment 41

17 years ago
*** Bug 95519 has been marked as a duplicate of this bug. ***

Comment 42

17 years ago
*** Bug 95974 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 43

17 years ago
*** Bug 97359 has been marked as a duplicate of this bug. ***

Updated

17 years ago
Blocks: 92997

Comment 44

17 years ago
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.

Comment 45

17 years ago
Created attachment 48024 [details] [diff] [review]
preliminary investigations

Comment 46

17 years ago
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. ***

Updated

17 years ago
Blocks: 99194

Comment 48

17 years ago
Is this a stop ship for nsbranch? Should we nsbranch+ it?

Comment 49

17 years ago
*** Bug 99745 has been marked as a duplicate of this bug. ***

Comment 50

17 years ago
*** 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 → ---

Comment 52

17 years ago
->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: nsbranch → nsbranch-
Priority: -- → P4
Target Milestone: --- → mozilla0.9.6

Comment 53

17 years ago
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?

Comment 54

17 years ago
*** Bug 101453 has been marked as a duplicate of this bug. ***

Comment 55

16 years ago
*** Bug 102859 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 56

16 years ago
dean was on the right track... just missed a similar spot in the slider code.
Status: NEW → ASSIGNED
(Assignee)

Comment 57

16 years ago
Created attachment 52421 [details] [diff] [review]
Patch for review/super-review.
Comment on attachment 52421 [details] [diff] [review]
Patch for review/super-review.

sr=ben@netscape.com
Attachment #52421 - Flags: superreview+

Comment 59

16 years ago
r=blake

Updated

16 years ago
Attachment #52421 - Flags: review+
(Assignee)

Comment 60

16 years ago
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 61

16 years ago
/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.")

Comment 62

16 years ago
I'll verify this, but I'm waiting to hear from Hyatt about my leak question.

Comment 63

16 years ago
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

Updated

16 years ago
Keywords: mozilla1.0, oeone
You need to log in before you can comment on or make changes to this bug.