418175 - popup attribute test fails if context menu is too big

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Andrew Schultz]

One on of my boxes, the native theme seems to space the context menu items out a lot and then context menu in test_popup_attribute is too tall to fit in the bottom half of the window (see attached screenshot).  This causes failures on multiple parts of the test.

The simple solution is to make the window taller.  620px is sufficient for me, but perhaps it ought to be 700px or something to ensure it works in cases where other things (large system font perhaps) cause similar problems.  The other solution would be to have the window resize itself based on the size of the popup.

Comment 1

[Andrew Schultz]

Attachment: screenshot

this from a point where the popup should be below

Comment 2

[Andrew Schultz]

Attachment: just make it taller

Comment 3

[Andrew Schultz]

Attachment: fix test_popup_button too

popup_button needs the same fix.  I also made the margins bigger to recenter the text/button.

Comment 4

[Neil Deakin]

Comment on attachment 304160 [details] [diff] [review]
fix test_popup_button too

This is the patch for some other bug.

Comment 5

[Andrew Schultz]

Attachment: the right patch

Comment 6

[Neil Deakin]

Comment on attachment 304245 [details] [diff] [review]
the right patch

Looks OK, but you will probably want to check if any of the test machines are running on a resolution 1024x768 or less. It's possible that the screen height minus the taskbar/menubar/etc + and window titlebar are less than 700 and would cause this to fail.

Comment 7

[Justin Dolske [:Dolske]]

The Windows test boxes are a little weird, in that the desktop inherits the properties of the RDP client that last connected to them (see: my headaches in bug 405384 and bug 414720).

It would probably be best to add an explicit check to |screen.width| and/or |screen.height|, and fail if you're below the minimum size. At least then the reason for the test failure will be obvious, after when someone (inevitably) connects at 640x400.

Comment 8

[Andrew Schultz]

OK, I landed the patch as-is and the tinderboxen didn't have a problem with it.  I'd be happy to add checks for resolution, although I'm not sure a) what min resolution to check for b) how to warn the user and/or get something into the logs about.

Comment 9

[Justin Dolske [:Dolske]]

I'd say:

1. Just pick a resolution that's known to work to start with.
2. Treat a lower resolution as a test failure.

I'd expect the effect of #2 to be that you'd see an error about needing a larger resolution, and then you'd see errors about the popup tests failing (because the screen really is too small).

Bonus points for trying the tests with your desktop at a couple different resolutions, to confirm that 800x600 or whatever is really too small. The check doesn't need to be precise to the exact dimension that starts to cause a failure.

Comment 10

[Andrew Schultz]

Attachment: test the resolution

For me, the popup attribute tests pass at 1024x768 and fails at 800x600.

Comment 11

[Andrew Schultz]

OK, resolution check is in too