84121 - Submenu of scrollable menu is positioned one menu row too low

Status: VERIFIED WORKSFORME

(Core :: XUL, defect, P2)

Description

[ajbu]

Mozilla build 2001060420, Windows 2000

I mentioned this before on bug 66848 (2001-03-31 06:46), filing this as a new 
bug after the checkin of bug 78409.
if a submenu of a scrollable menu is 'bounced' of the bottom of the screen, or 
the 
submenu is so large it is a scrollable submenu itself, it is positioned (one 
menurow?) to low.

So a big scrollable submenu (larger then the height of the screen is positioned 
(top=1, bottom=screenheight+1, height=screenheight) instead of (top=0, 
bottom=screenheight).
Submenu’s aligned to the bottom of the screen are aligned to (screenheight+1) 
instead screenheight. This seems independent of the scrolled position of the 
scrollable menu.

Steps to reproduce: 
1) Create a bookmark menu so large it becomes scrollable
2) Create a submenu of that scrollable menu so this would be large enough to go 
beyond the bottom of the screen
3) Open the bookmark menu and the submenu

Actual results: The submenu is positioned to low, the last row of the submenu 
is offscreen
Expected results: Submenu completely visible on the screen.


See the screenshots:
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=29376
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=29375
(Screenshots where created with an earlier build, but seeing the same in build 
2001060420)

Comment 1

[Mike Pinkerton (not reading bugmail)]

ajbu, can you attach a bookmarks file that exhibits this? that'll make it easier
to debug and fix.

Comment 2

[ajbu]

Attachment: Bookmark file large enough to be scrollable. Submenu is shifted down (Bookmarkfile size might depend on screen resolution/fonts)

Comment 3

[selmer (gone)]

Does this mean that the incorrectly positioned menu can have items that are not
selectable?

Comment 4

[ajbu]

On my screen the top two pixels of the last menu item (or the scroll arrow if 
that menu is scrollable itself) are visible, so it can be selected but the text 
of that item is not visible so this might not be very usefull. The item can 
also be reached with the cursur keys.

Comment 5

[ajbu]

Note that the size of the bookmark list needed to make it scrollable depends on 
the screen and font settings.
The effects of bug 75142 (when opening a bookmarks menu that needs scroll 
arrows, ALL parent menus get scroll arrows ) might also give insight:
When making the bookmark menu smaller so it fits on the screen, 'Test Folder' 
is positioned correctly. When opening an submenu that needs scroll arrows, the 
bookmark menu is also given scroll arrows, and 'Test Folder' is shifted down.

Comment 6

[Mike Pinkerton (not reading bugmail)]

over to ben.

Comment 7

[John Morrison]

marking new. As noted above, all items on the bookmarks can be selected, but 
the bottom-most item on a submenu is partially off the bottom of the screen,
and the text is generally not readable (win32/linux/mac(personal toolbar 
bookmarks, top level OK) on both branch and trunk). It's worse in Classic
than in Modern (where it is partially readable).

Comment 8

[Paul Chen]

*** Bug 84387 has been marked as a duplicate of this bug. ***

Comment 9

[nhottanscp]

Attachment: Screen shot, scroll problem in mail folder property dialog.

Comment 10

[Frank Tang]

> Naoki Hotta wrote:
>     This has to be fixed for RTM. Looks like a xptoolkit problem.
Yung-Fong Tang wrote:
> why?

Naoki Hotta wrote:
The problem is not only the arrow is missing but also you cannot see 
what is selected, so it is realy hard to select the right charset (i.e. 
the feature is not really functioning).

Comment 11

[doctor__j]

*** Bug 84777 has been marked as a duplicate of this bug. ***

Comment 12

[Jaime Guerrero at iPlanet, a Sun+Netscape Alliance]

removed myself from this bug as I was CC'd in error (wrong email address)

Comment 13

[nhottanscp]

sorry about the mistake, adding [e-mail] to cc.

Comment 14

[Jacek Piskozub]

*** Bug 85180 has been marked as a duplicate of this bug. ***

Comment 15

[Jaime Rodriguez, Jr.]

Nhotta - Is this i18n issue, or a CORE issue?

Comment 16

[nhottanscp]

Jaime, this is a core issue but it affects i18n feature (mail folder property to
set a folder charset shown in the attachment on 06/08/01 10:14).

Comment 17

[marina]

in 2001-06-11-04 build i am not able to use up and down arrows to scroll in a
Folder charset dlgbox as a workaround, it makes folder charset practically
unusable ( since this bug was introduced we at least had a workaround) 

Comment 18

[Jaime Rodriguez, Jr.]

Changing the severity to major. This bug does not allow users to set their
character encoding by mail folder. However, we can note the workaround, whish is
to scroll with the up and down keys, as oospoossed to the mouse.

Nominate fror M0.9.3 | P2. 

Vishy - Let's try and get this one, if all other work has been completed for
M0.9.2. Pls set priority to P2.

Comment 19

[marina]

correctness with 2001-06-12-04 build:i am able to navigate again with up and
down arrows (meaning that we still have a workaround...) and can use the folder
charset... though it would be good to fix it :-)

Comment 20

[Paul Chen]

nav triage team:

Marking nsbeta1+ and p2. Note, on a mac commercial build, the menu is drawn past
the bottom of the screen, so if you move the cursor below the bottom of the last
menu item, you can scroll the menu down.

Comment 21

[Viswanath Ramachandran]

nav triage: not critical for netscape 6.1 rtm, -> m1.0. 

Comment 22

[:deanis74]

*** Bug 89112 has been marked as a duplicate of this bug. ***

Comment 23

[:deanis74]

From 89112, attachment 41087 [details] has another screenshot.

Comment 24

[Jacek Piskozub]

*** Bug 89752 has been marked as a duplicate of this bug. ***

Comment 25

[[e-mail]]

*** Bug 89867 has been marked as a duplicate of this bug. ***

Comment 26

[Mike Kowalski]

*** Bug 93796 has been marked as a duplicate of this bug. ***

Comment 27

[ji]

Due to this problem, it makes folder charset UI very hard to use (see this 
screen shot 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=37665).
Nominating for nsbranch.

Comment 28

[:deanis74]

*** Bug 95738 has been marked as a duplicate of this bug. ***

Comment 29

[Mike Kowalski]

*** Bug 87533 has been marked as a duplicate of this bug. ***

Comment 30

[Jacek Piskozub]

Marking mostfreq at 10 dups.

Nominating mozilla0.9.5 as this is a very visible bug.

Comment 31

[Jaime Rodriguez, Jr.]

Pchen - This is getting a lot of dupes reported against it. I agree with Jacek
nomination for M0.9.5. Let's fix this one.

Comment 32

[Paul Chen]

ok, we'll try to get this done for 0.9.5 but no guarantees, marking mozilla0.9.5

Comment 33

[:deanis74]

I may like mucking with menus, but Jan's better-versed with these scrolling
suckers.  Jan, any idea on this one?

Comment 34

[Jan Varga [:janv]]

Menupopup probably doesn't know about up and down arrow

Comment 35

[Jan Varga [:janv]]

some debug info:
method GetViewOffset() returns y value like this:
360 - when bookmarks menu is not scrollable
600 - when bookmarks menu is scrollablethis value is used to position submenu

Comment 36

[Jan Varga [:janv]]

I think Ben understands this code much better.

Comment 37

[Ashley Bischoff (blog at handcoding.com)]

*** Bug 96905 has been marked as a duplicate of this bug. ***

Comment 38

[Jaime Rodriguez, Jr.]

Ben - What's the status on this one? We'd like to have afix ASAP (i.e. No late
then the end of the week). If this can be completed by Friday, please work with
you Manager to get this one nsbranch+.

Comment 39

[Cindy Roberts]

Marking nsbranch- as it was decided in the August bug triage that we wouldn't
have eenough time in eMojo to fix this.  Let's revisit for MachV.

Comment 40

[Peter Trudelle]

removing redundant nsbranch keyword, ->0.9.6

Comment 41

[Frank Tang]

please also see 100986

Comment 42

[marina]

this is a very important fix from the intl point of view, the folder properties
( its charset) can not be changed, the workaround would be to use up and down
arrow keys but this is not that evident to every user, more over we started to
get bugs where people are complaining about they Inbox being blank because the
encoding suddenly switched to Arabic and they are not able to change it. it
might happen that this new bug is not caused by 84121 but without this one being
fixed we can not verify that.

Comment 43

[bclinger]

Your workaround does not consistently "fix" the display problem.

Comment 44

[marina]

even more reason to fix this bug!

Comment 45

[ji]

Nominating for nsbeta1 since it's very inconvenient for users to select a folder
charset from View | Folder Character Coding. 

Comment 46

[Mike Kowalski]

*** Bug 88555 has been marked as a duplicate of this bug. ***

Comment 47

[:deanis74]

Adding relnote keyword.  This is a pretty frequent bug, so we should mention
that we know about it.

Comment 48

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

There were a couple problems at work here.

1. The ySpillage wasn't taking into account the fact that the menu is offest
down in the y direction by the height of the combobox.

2. The wrong value was being used for the size of the screen (screenheight
should be used)

I also fixed another bug where if you moved the dialog off screen and popped
down the menu, it was bigger than the screen.

This case should have checking for bigger than screen as well.

Looking for review.

Comment 49

[Jacek Piskozub]

The fix has an mismatched parenthesis. I'll post the corrected file. Otherwise
looks OK. 

Comment 50

[Jacek Piskozub]

Attachment: Fix which actually compiles (mismatched parenthesis deleted)

Comment 51

[Jacek Piskozub]

Attachment: I posted the wrong file. Now the fix is *really* corrected

Comment 52

[:deanis74]

Kinda makes sense to me, I guess.  Does it fix the problem with the submenus of
scrollable menus (screenshots in initial report and testcase is the first
attachment), as well as the menulist problem that brought you here?

Comment 53

[Jacek Piskozub]

I applied the patch to my CVS tree (WindowsME). Seems it cures all the problem
of vanishing lower ends of 2nd tier bookmark lists and submenus (like Message ->
Move -> or View -> Encode ->) 

Comment 54

[:deanis74]

r=[e-mail], assuming you've tested the heck out of it.

Comment 55

[Jacek Piskozub]

As I test this for two days with no side effect, like the logic behind the
changes, and my input to the patch is only one character deleted, I believe I
can give it a second positive review (r=[e-mail])

Comment 56

[Jacek Piskozub]

Adding hyatt to CC: hoping for a sr= review (I believe the only peer of this
component who has sr= priviledges)

Comment 57

[R.K.Aa.]

*** Bug 85409 has been marked as a duplicate of this bug. ***

Comment 58

[nhottanscp]

*** Bug 115188 has been marked as a duplicate of this bug. ***

Comment 59

[Chris Lyon]

*** Bug 117152 has been marked as a duplicate of this bug. ***

Comment 60

[:deanis74]

hyatt, blake, ben...  can one of you sr=?  This is a fairly visible bug.

Comment 61

[Chris Lyon]

*** Bug 97551 has been marked as a duplicate of this bug. ***

Comment 62

[R.K.Aa.]

*** Bug 118107 has been marked as a duplicate of this bug. ***

Comment 63

[John Morrison]

*** Bug 118505 has been marked as a duplicate of this bug. ***

Comment 64

[Jacek Piskozub]

Any chance for sr=? This bug has already 23 dups :-(

Comment 65

[David Hyatt]

sr=hyatt.

Comment 66

[Boris Zbarsky [:bzbarsky]]

checked in

Comment 67

[:deanis74]

This doesn't look like quite the right fix.  Using the testcases in bug 100986,
the first menu isn't long enough and the second two are still too long.  Plus,
my scrolling bookmarks menu is one row too short, similar to the first testcase
in 100986.

Comment 68

[Mike Kaply [:mkaply]]

This does fix the original case in 100986 (the charset menu in mail folder 
properties) but itdoes not fix the testcases.

The testcase might be another issue.

Comment 69

[:deanis74]

Yes, it fixes that one menulist.  And it fixes the large scrolling sub-menu of
my bookmarks menu, which was the original report in this bug.  But my scrolling,
top-level bookmarks menu is now one row too short.

Comment 70

[bclinger]

It is still too low in a couple of places in my bookmarks file. It is better,
but still not fixed.

Comment 71

[Erich 'Ricky' Iseli]

*** Bug 119811 has been marked as a duplicate of this bug. ***

Comment 72

[Mike Kowalski]

I still see this. The menu goes too far down and I can't read the last menuitem.

Comment 73

[[e-mail]]

In 0.9.8, I'm not only getting the top level menu one line too short, but the
other menus are still one line too far down--just in a different way.  The extra
line, instead of just falling off the bottom of the screen, causes a scroll
arrow to appear and lets me scroll down by 1 line.  (The menus are short enough
that if they were properly positioned on the screen, they wouldn't need scroll
arrows.)

The scroll arrow itself is almost at the right spot, so if the last line was
simply placed where the scroll arrow is, it would be nearly correct...

Comment 74

[Peter Trudelle]

nsbeta1- per Nav triage team.

Comment 75

[Mike Kowalski]

It seems like if the menu width is narrow and you have a lot of bookmarks, it
doesn't give you arrows unless you have a certain amount of bookmarks. It then
hides the submenu below the screen while it should move it up to fit on the
screen. Anyone else see this problem? I think this bug is easy to notice and
since just about everyone uses the bookmarks menu that this bug is considered
very important to Mozilla1.0.

Comment 76

[Chris Lyon]

*** Bug 129572 has been marked as a duplicate of this bug. ***

Comment 77

[Mike Kowalski]

This is an example bookmark file that I created that shows the bug.

Comment 78

[Mike Kowalski]

Attachment: this bookmark file shows bug

Comment 79

[Walter K.]

Attachment: Screenshot of scroll menu with 1 item

I confirm comment #73 on W2K, build 2002041206. When menu touches windows'
Taskbar, it creates up/down scroll arrows for 1 item. Instead, menu should
display one more line. See attached screenshot.

Comment 80

[Stephan Hohe]

I see the same on Win 98 SE; Moz 1.0 RC1 and i've seen it quite a while as shown
in attachment 79046 [details].
The displayed menu shows scroll arrows where it would not be necessary, actually
producing a larger menu then if showing it without any scrolling.
I would think this is an other bug then the original one, but I don't know if
it's related. Maybe someone should change the summary.

Comment 81

[:deanis74]

Attachment 79046 [details] is bug 135076.  There's a patch there, and I wonder if it might
end up fixing this bug as well.  That patch should be checked in any time now,
so we'll know the answer soon.

Comment 82

[Mike Kowalski]

Just tested Mozilla 1.0 RC3 and this bug has been fixed. Marking WORKSFORME.

Comment 83

[bclinger]

I downloaded Mozilla build 2002052404 and tested for this bug. The bug is still
here in the same form. There are still numerous instances when text is not fully
displayed. So, it is still not fixed and does not work for me.

Comment 84

[:deanis74]

Really?  I imported the large bookmarks file (attachment 37223 [details]) into my
bookmarks.  The main Bookmarks menu displays properly, as does the "Test Folder
(big)" submenu.  This is off of both the Bookmarks menu and the Bookmarks button
on the personal toolbar.

Comment 85

[:deanis74]

Is this still an issue?  mkaply fixed something related to this a while back.

Comment 86

[Mike Kowalski]

This is fixed.

Comment 87

[:deanis74]

Verified.  I can't reproduce this anymore.

Comment 88

[Kathleen :Brade]

*** Bug 118015 has been marked as a duplicate of this bug. ***