418798 - Preference window tooltips are squashed horizontally

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[:Gijs (he/him)]

This is a regression on trunk. Reproduced on:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120610 XULRunner

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9a9pre) Gecko/2007110318 XULRunner

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012504 Minefield/3.0b3pre

(windows + mac trunk xulrunner, windows trunk Fx)


The tooltips are very narrow and hence very high. I'm wondering what's up, if the units are wrong or if the x and y are reversed, or something else still. I'm very sure this used to work, though! And of course, this is really a trunk bug, but I'm not sure where this goes yet, so filing it on us for now...

Comment 1

[James Ross]

We call the popup box object's sizeTo(width, height) with the width from the box object of the element we're showing the tooltip for (well, its container). It's either getting the wrong element (somehow only on trunk) or, my bet, they've totally shagged the box object properties and methods on trunk (I'd default to blaming the page-zoom stuff, which I know replumbed a lot of internal things). I guess it's also possible it is ignoring the sizeTo call, but I have no idea what crazy nutcaze would have made it be so narrow by default.

Comment 2

[Damon Sicore (:damons)]

Screen shots?  Test case?  Those would really help.

Comment 3

[James Ross]

See the bug's URL, dammit. I can attach as many images as you like showing this, but there is already one availabe.

Comment 4

[Damon Sicore (:damons)]

Sorry, didn't see the url.  Sheesh.

Comment 5

[:Gijs (he/him)]

OK, so I checked, and it looks like sizeTo is broken.

Note:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/extensions/irc/xul/content/config.js&rev=1.13?mark=927,930#910

Hovering over the Global Settings > General > Enable ident server pref, and alerting tipobjBox.width gives back 339 on both branch and trunk, but on branch the popup has the appropriate width, and on trunk it does not. Punting to Toolkit > XUL Widgets, noting that mconnor has already requested blocking1.9?

Comment 6

[:Gijs (he/him)]

It turns out that tt.boxObject.height is actually 0. (38 on branch)
I'm guessing that's why sizeTo does not "work". On branch, however, the height simply gets ignored, and the width is still correctly set. I looked at

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/xul/base/src/nsPopupBoxObject.cpp&rev=1.28#159
(which I think implements this? Please correct me if I'm wrong)
and can't figure out why that'd fail for 0 height all of a sudden. Also, maybe my assessment was wrong and this bug needs to be in Layout somewhere? Someone who knows, please help out! :-)

On the other hand, I would be curious to know why the height is 0 on trunk, and would guess that's a separate regression? :-(

Comment 7

[:Gijs (he/him)]

So comment #5 is wrong. The problem isn't sizeTo, it's the boxObject's height/width stuff. For some insane reason (probably to do with old core bugs) ChatZilla resizes again onpopupshowing:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/extensions/irc/xul/content/config.js&rev=1.13&mark=1304-1305,1313#1298

So I tested these values against
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031205 Minefield/3.0b5pre

I alerted, in order, popup.boxObject.width, diff, fChild.boxObject.height.
Branch: 339, 6, 45
Trunk: 0, 0, 0

Sooo... this shouldn't be in XUL Widgets, I think, but I have no idea where it does belong. Could someone lend a hand? I'll try getting a regression range.

Comment 8

[Neil Deakin]

The size of a popup before it is open is 0.

Various bugs were fixed in 1.9 so that tooltips get sized properly. You shouldn't have to set the size at all.

Why are you opening this tooltip manually?

Comment 9

[Neil Deakin]

(In reply to comment #8)

> Why are you opening this tooltip manually?
> 

And more importantly, if this code is used to show the tooltip in the screenshot, why is it manually handling the mouse events to show the tooltip?

Comment 10

[James Ross]

How the tooltip is opened is irrelevant to this bug, but it's highly likely that the "official" ways all failed to do what we wanted with respect to opening it.

The sizing is what's at issue here, and the automatic sizing definately doesn't do what we want.

Comment 11

[James Ross]

Attachment: Work around trunk behaviour changes

If the popup box object width is 0, we're on trunk (and also only the first time through this code), so we set the flag; not calling sizeTo here would almost be enough, but as the text displayed varies, the tooltip never shrinks by itself - setting height=0 forces it to expand to just the right size.

Tested on Mozilla 1.2, 1.4, 1.6, 1.7, SeaMonkey 1.0, 1.1, Firefox 1.0, 1.5, 2.0, and current trunk.

Mozilla 1.0 throws an error on the sizeTo call in the other part of the code, and I didn't feel like fixing that here.

Comment 12

[:Gijs (he/him)]

Comment on attachment 308947 [details] [diff] [review]
Work around trunk behaviour changes

+    if (popup.boxObject.width == 0)

Nit: please add a 1-2 line comment above this line to indicate what we're hacking around. Otherwise, given that you've tested this on just about every stable build on the planet, r=me.

Comment 13

[James Ross]

Checked in --> FIXED.

Comment 14

[:Gijs (he/him)]

I didn't mean to submit the topic and dependency edits. Bugzilla was smart enough to keep the edit field values on refresh after a mid-air, and hide them again, so I didn't know I wasn't *just* clearing blocking1.9. Looks like I'll be filing another bug soon...

My apologies for the mess.

Comment 15

[Neil Deakin]

Did you actually try removing the manual sizing code, which is the correct fix?

The reason I ask is that we have an automated test (test_tooltip.xul) that opens tooltips and inserts/removes text from it, and it resizes fine.

Comment 16

[:Gijs (he/him)]

(In reply to comment #15)
> Did you actually try removing the manual sizing code, which is the correct fix?
> 
> The reason I ask is that we have an automated test (test_tooltip.xul) that
> opens tooltips and inserts/removes text from it, and it resizes fine.
> 

Trunk does seem OK here. Branch doesn't display a popup at all without sizeTo calls, though.

Comment 17

[James Ross]

The height seems to be persisted as a result of the sizeTo at
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/extensions/irc/xul/content/config.js&rev=1.14&mark=931#911
using the box object's height, and then being unable to correct to the new height in the onpopupshowing event due to the Gecko 1.9 changes. Short of checking for a 0 boxObject.height during setTooltipState and branching *more* code on the result, I don't see how we'd continue working on even Firefox 2.0.

If you want to try to find a cleaner solution that works on all 11+ hosts, feel free. I'd rather have an incremental change to cope with the popup event changes now, than leaving it unfixed for ages because no-one can be bothered to rewrite the entire section of code.