Closed Bug 1665476 Opened 4 years ago Closed 2 years ago

LW-theme background images aren't shown

Categories

(Core :: Layout, defect)

defect

Tracking

()

RESOLVED FIXED
107 Branch

People

(Reporter: Paenglab, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files)

LW-theme background images do not show any more.

Back-out of bug 1665064 fixes it for me.

Emilio, do you know why this happens? I CCed Magnus if you have questions about deeper functions.

Flags: needinfo?(emilio)

What are LW-theme background images? Can you attach an screenshot?

(No idea why off-hand, we should only skip painting backgrounds during print).

Flags: needinfo?(richard.marti)
Attached image LW_theme.png

This screenshot shows the LW-theme "Dark Fox" applied. In front the smaller window how it should look and on background how it looks now.

Maybe this happens on TB because TB doesn't use electrolysis.

Flags: needinfo?(richard.marti)

I'm a complete noob in Cpp so I don't really know what I'm doing. With a back-out of only the changes in nsDisplayList.cpp the LW-theme image is shown again. A back-out of only the changes in nsCanvasFrame.cpp shows still the issue.

Emilio, not sure if that helps you.

Sorry for the lag in looking into this. So if I back out my patch, on a debug build I get:

Assertion failure: mFrame->IsCanvasFrame() || mFrame->IsFrameOfType(nsIFrame::eTablePart), at /home/emilio/src/moz/gecko-5/layout/painting/nsDisplayList.cpp:3355

So something is a bit off...

Assignee: nobody → emilio
Flags: needinfo?(emilio)

It's unused on mozilla-central, and Thunderbird can just use the canvas
frame as regular (X)HTML documents, so just use a canvas frame instead
of an nsRootBoxFrame for XUL as well.

nsRootBoxFrame was needed because of various XUL-specific things like
tooltips and so on lived there. But with the move away from XUL, that
functionality has been added to nsCanvasFrame already, behind a
principal check instead.

This also allows simplifying our background propagation setup, which was
only half-working for XUL documents (this bug is a consequence of that).

With this, most of the callers of nsCSSRendering::IsCanvasFrame can go.
They're only two of the frames that would return true for that that
actually paint backgrounds (nsCanvasFrame and nsRootBoxFrame), so the
codepaths in display list building and painting can just check
frame->IsCanvasFrame() instead.

The remaining caller to that function is
nsContainerFrame::SyncWindowProperties, and the change is also legit, in
the sense that the only thing SyncWindowProperties() really cares about
is propagating the max/min-width constraints from the root element's
style to the view/widget, and the only frame that would return true from
IsCanvasFrame and have a view is the viewport frame which is the root of
the frame tree.

I tried your patch on Linux and Windows on TB.

  • On both platforms the Add-on manager fills only the top fifth of the window.
  • On both platforms the DevTools Toolbox (Browser Toolbox on FX) fills also only the top fifth of the window.
  • On Linux the theme background image is displayed but with the dark or light theme the titlebar and tabbar are transparent.
  • On Windows first the theme background looked good but after a restart of TB the whole window is transparent. Also in safe mode no change. The safe mode dialog is also transparent but the text is still visible.

Alright, I'll look into that, there's also a bit of orange in my try push.

Flags: needinfo?(emilio)

The updated patch should be much closer if you want to give it a run. I still need to fix one or two existing xul tests, but won't get to that today.

Flags: needinfo?(richard.marti)

On Linux all is looking good but Windows still shows a completely transparent window. Absolute nothing is visible. When I start in safe mode, the safe mode dialog is shown with the content but no window decoration (no titlebar, no window border).

Flags: needinfo?(richard.marti)

Can you check on a debug build to see if there's any assertion going on?

Flags: needinfo?(richard.marti)
Depends on: 1666497
Attached file console.log

I attached the log when starting with mach run. Before I built without the patch and correlated both outputs. No additional assertions are in the log.
But with your patch I see this line:
[GFX2-]: ASurface Init failed with Cairo status 1 on 00007FFDB7D0A660
and this:
WARNING: GetImageSurface on an invalid (null) surface; who's calling this without checking for surface errors?: file z:/Mozilla/comm-central/gfx/thebes/gfxWindowsSurface.cpp, line 149

Flags: needinfo?(richard.marti)

Emilio, can my log and comment 11 help you find the issue?

(In reply to Richard Marti (:Paenglab) from comment #14)

Emilio, can my log and comment 11 help you find the issue?

Yes, I know what the issue is already, it was just harder to fix than it seemed, I just got all the dependencies sorted out and can try to land this again.

My patch somehow causes WR to crash and fall back to d2d on Windows, see
bug 1671591.

While I figure that out let's fix TB by restoring the previous behavior
for XUL.

Rename the Root frame type because it's super-confusing that IsRootFrame
is not equivalent to !GetParent().

Can you confirm comment 16 fixes it for you too? The proper fix will take a bit more because of bug 1671591 and me being sick this week :(

Flags: needinfo?(emilio) → needinfo?(richard.marti)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #17)

Can you confirm comment 16 fixes it for you too? The proper fix will take a bit more because of bug 1671591 and me being sick this week :(

Yes, this fixes the background image issue. Many thanks.

Flags: needinfo?(richard.marti)
Attachment #9182615 - Attachment description: Bug 1665476 - More targeted fix. r=mats,#layout-reviewers → Bug 1665476 - Handle XUL root frame backgrounds explicitly in AppendBackgroundItemsToTop. r=mats
Keywords: leave-open
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cafeb3cc35d9
Handle XUL root frame backgrounds explicitly in AppendBackgroundItemsToTop. r=mats

It seems like nowadays the WR failure is much noisier, and I should be able to debug it better... https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=E-0RYZy_SR6LwPIc6gjgGA.0&revision=92edd731e64cc448719e8dac6ae573efda121463

(In reply to Emilio Cobos Álvarez (:emilio) from comment #23)

It seems like nowadays the WR failure is much noisier, and I should be able to debug it better... https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=E-0RYZy_SR6LwPIc6gjgGA.0&revision=92edd731e64cc448719e8dac6ae573efda121463

Bug 1671591 is P3, so perhaps won't get attention soon.

Severity: -- → S3
See Also: → 1671591
Version: unspecified → Thunderbird 82

I checked and bug 1671591 doesn't seem to keep happening, so I'll rebase and reland.

Backed out changeset e41bfdf79fb8 (Bug 1665476) for causing windows mochitest failures in test_windowminmaxsize.xhtml
Backout link: https://hg.mozilla.org/integration/autoland/rev/54f245b73e54063142fb0d7e1a493401f6389cfd
Push with failures, failure log.
(Update): Looks to have also caused bc failures.

Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Attachment #9176854 - Attachment description: Bug 1665476 - Remove nsRootBoxFrame to unify background propagation between XUL and non-XUL documents. r=#layout-reviewers → Bug 1665476 - Remove nsRootBoxFrame to unify background propagation between XUL and non-XUL documents. r=mats
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/503c84054f68
Remove nsRootBoxFrame to unify background propagation between XUL and non-XUL documents. r=layout-reviewers,mats
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/37180b0bf14a
Remove nsRootBoxFrame to unify background propagation between XUL and non-XUL documents. r=layout-reviewers,mats
Flags: needinfo?(emilio)
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Regressions: 1792809
Regressions: 1793287
Regressions: 1793402
Component: General → Layout
Keywords: leave-open
Product: Thunderbird → Core
Version: Thunderbird 82 → unspecified
Target Milestone: --- → 107 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: