Closed Bug 970788 Opened 10 years ago Closed 10 years ago

UITour: [Linux] Visual panel bugs when highlighting targets due to incorrect panel size

Categories

(Firefox :: General, defect, P2)

29 Branch
All
Linux
defect

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: jgmize, Assigned: MattN)

References

()

Details

(Whiteboard: [Australis:P?][kb=1271327])

Attachments

(5 files, 1 obsolete file)

The tour looks great overall, very smooth, nice work!

I did notice a few issues on the Linux x86_64 29.0a2 (2014-02-10)-- please see the attached screenshots.
I can see the same issue on the Linux i686 build.
Whiteboard: [kb=1271327]
These appear to be visual glitches in the Firefox UITour, probably move this to the Firefox product component?
Blocks: fx-UITour
Component: Bedrock → General
Product: www.mozilla.org → Firefox
Summary: Visual bugs on https://www.mozilla.org/en-US/firefox/29.0a2/whatsnew/?oldversion=28.0a2 → UITour: [Linux] Visual bugs
Target Milestone: --- → Firefox 30
Version: Production → 29 Branch
Summary: UITour: [Linux] Visual bugs → UITour: [Linux] Visual bugs when highlighting in browser chrome
Priority: -- → P2
Hardware: x86_64 → All
Hi Raymond-

Could you please check and see if you can reproduce this?  And talk with MattN (and/or jgmize) about it?

Thx,
Jen
Flags: needinfo?(mozbugs.retornam)
This bug is valid.
Flags: needinfo?(mozbugs.retornam)
Thanks, Raymond!

Matt - could you please follow up to see how Raymond reproduced it?
I can't reproduce on Fedora 18 with the Gnome Shell.

Can someone provide more details to reproduce the problem?

* Any specific steps to reproduce?
* Is it reproducible 100% of the time?
* What are details about the environment? Distro? Version? Window Manager? etc.
* A regression range with mozregression would be helpful
Flags: needinfo?(mozbugs.retornam)
Summary: UITour: [Linux] Visual bugs when highlighting in browser chrome → UITour: [Linux] Visual panel bugs when annotating targets
(In reply to Matthew N. [:MattN] from comment #8)
> I can't reproduce on Fedora 18 with the Gnome Shell.
> 
> Can someone provide more details to reproduce the problem?
> 
> * Any specific steps to reproduce?
> * Is it reproducible 100% of the time?
> * What are details about the environment? Distro? Version? Window Manager?
> etc.
> * A regression range with mozregression would be helpful

I'm running Firefox on Ubuntu 13.04 with Unity enabled. This happened a couple of times after I downloaded and started Linux x86_64 29.0a2 (2014-02-10) build
Flags: needinfo?(mozbugs.retornam)
I installed Ubuntu and can now reproduce. I'll start by figuring out why the shape is wrong.
Assignee: nobody → MattN+bmo
Status: NEW → ASSIGNED
Target Milestone: Firefox 30 → ---
The problem here is that after we change the dimensions of the highlight panel (e.g. to 24 x 24), nsXULPopupManager::PopupResized ends up getting called with the following stack:
#0  nsXULPopupManager::PopupResized (this=0x7f29ce682aa0, aFrame=0x7f29b0fb1400, aSize=...) at /mozilla-central/layout/xul/nsXULPopupManager.cpp:410
#1  0x00007f29df3e7856 in nsView::WindowResized (this=0x7f29bada0940, aWidget=0x7f29b0654760, aWidth=26, aHeight=66) at /mozilla-central/view/src/nsView.cpp:1010
#2  0x00007f29ded82d5e in nsWindow::DispatchResized (this=0x7f29b0654760, aWidth=26, aHeight=66) at /mozilla-central/widget/gtk/nsWindow.cpp:449
#3  0x00007f29ded879c8 in nsWindow::OnSizeAllocate (this=0x7f29b0654760, aAllocation=0x7fff96339020) at /mozilla-central/widget/gtk/nsWindow.cpp:2421
#4  0x00007f29ded8ec99 in size_allocate_cb (widget=0x7f29bdd5b010, allocation=0x7fff96339020) at /mozilla-central/widget/gtk/nsWindow.cpp:5189
#5  0x00007f29dbb00188 in g_closure_invoke () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0

You can see that nsWindow::DispatchResized is getting called with aWidth=26, aHeight=66 and the output of  printf("%d, %d, %d, %d\n", curDevSize.width, aSize.width, curDevSize.height, aSize.height) is:
24, 26, 40, 66

So curDevSize.width matches what I explicitly set the height and width attributes of the panel to, but the other three are different. This leads to the width and height attributes getting set to 26 x 66 which is a very wrong height and a somewhat wrong width.

I have some questions:
a) How is the size 26 x 66 being calculated? Both the panel and its only child have explicit dimensions which should lead to a size of 24 x 24.
b) Why are we overriding the width and height attributes already set on the panel? I think this is related to bug 809388 comment 11.
Summary: UITour: [Linux] Visual panel bugs when annotating targets → UITour: [Linux] Visual panel bugs when highlighting targets due to incorrect panel size
The problem here is that after we change the dimensions of the highlight panel (e.g. to 24 x 24), nsXULPopupManager::PopupResized ends up getting called with the following stack:
#0  nsXULPopupManager::PopupResized (this=0x7f29ce682aa0, aFrame=0x7f29b0fb1400, aSize=...) at /mozilla-central/layout/xul/nsXULPopupManager.cpp:410
#1  0x00007f29df3e7856 in nsView::WindowResized (this=0x7f29bada0940, aWidget=0x7f29b0654760, aWidth=26, aHeight=66) at /mozilla-central/view/src/nsView.cpp:1010
#2  0x00007f29ded82d5e in nsWindow::DispatchResized (this=0x7f29b0654760, aWidth=26, aHeight=66) at /mozilla-central/widget/gtk/nsWindow.cpp:449
#3  0x00007f29ded879c8 in nsWindow::OnSizeAllocate (this=0x7f29b0654760, aAllocation=0x7fff96339020) at /mozilla-central/widget/gtk/nsWindow.cpp:2421
#4  0x00007f29ded8ec99 in size_allocate_cb (widget=0x7f29bdd5b010, allocation=0x7fff96339020) at /mozilla-central/widget/gtk/nsWindow.cpp:5189
#5  0x00007f29dbb00188 in g_closure_invoke () from /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0

You can see that nsWindow::DispatchResized is getting called with aWidth=26, aHeight=66 and the output of  printf("%d, %d, %d, %d\n", curDevSize.width, aSize.width, curDevSize.height, aSize.height) is:
24, 26, 40, 66

So curDevSize.width matches what I explicitly set the height and width attributes of the panel to, but the other three are different. This leads to the width and height attributes getting set to 26 x 66 which is a very wrong height and a somewhat wrong width.

I have some questions:
a) How is the size 26 x 66 being calculated? Both the panel and its only child have explicit dimensions which should lead to a size of 24 x 24.
b) Why are we overriding the width and height attributes already set on the panel? I think this is related to bug 809388 comment 11.

Perhaps this is also related to bug 788829? tn and Enn, it seems like you two are familiar with this code so could you please try to take a look at this? We're trying to get our Australis P1-P3 bugs closed this week.
Flags: needinfo?(tnikkel)
Flags: needinfo?(enndeakin)
Yeah Linux and popups is kinda broken. I remember debugging one issue for the devtools folks where X would send back some bounds pulled out of nowhere, and gecko was just supposed to ignore that and wait for X to get with the program and resize the popup to the correct size. But there was no easy way to tell if that notification with strange bounds should have been ignored. The devtools folks found a workaround in js before we could wrestle X into submission. That might be your best bet.

Karl, do you remember if we had a reasonably workable plan for this problem?
Flags: needinfo?(tnikkel) → needinfo?(karlt)
It sounds like the width and height of the panel may be getting changed independently.  If that is what is happening then there could be two resized notifications.  These will arrive *after* the first resize and may arrive after both resizes, but the last size received should be correct.

Gecko must not request a new size to match a size received in a resized event, which might be what bug 809388 comment 11 is about.

That still doesn't explain why the widths differ.  Could there be a minimum size restriction somewhere?
Flags: needinfo?(karlt)
This leaks a docshell in the test but seems to work well enough. Definitely better than nothing.
Attachment #8387156 - Flags: feedback?(gijskruitbosch+bugs)
Comment on attachment 8387156 [details] [diff] [review]
Mutation Observer Workaround v.1 - Leaks a docshell

Review of attachment 8387156 [details] [diff] [review]:
-----------------------------------------------------------------

Approach seems fine, worried about leaks. :-(
Attachment #8387156 - Flags: feedback?(gijskruitbosch+bugs) → feedback+
Apparently a large number of the UITour bugs in the directory already leak 1 docshell when run individually so I filed bug 980746 to track down the issue.

Try push: https://tbpl.mozilla.org/?tree=Try&rev=3c41fc3f72a6
Attachment #8387156 - Attachment is obsolete: true
Attachment #8387331 - Flags: review?(gijskruitbosch+bugs)
Attachment #8387331 - Flags: review?(gijskruitbosch+bugs) → review+
Attachment #8386113 - Flags: checkin-
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [kb=1271327] → [kb=1271327][fixed-in-fx-team]
Comment on attachment 8387331 [details] [diff] [review]
Mutation Observer Workaround v.1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 936187
User impact if declined: Highlights on Linux are incorrectly sized and look quite bad
Testing completed (on m-c, etc.): Locally with Ubuntu and m-c
Risk to taking this patch (and alternatives if risky): Linux-only risk for repaint flicker but is less bad that the problem it fixes.
String or IDL/UUID changes made by this patch: None
Attachment #8387331 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a48993c2af06
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [kb=1271327][fixed-in-fx-team] → [kb=1271327]
Target Milestone: --- → Firefox 30
Attachment #8387331 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [kb=1271327] → [Australis:P?][kb=1271327]
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: