991542 - Odd shadowing around newtab's sponsored panel

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Comment 1

[Ed Lee :Mardak]

Not sure what happened to my description...

Bug 975228 comment 41 item 4 mentions an odd shadowing on Windows and provides attachment 8401126 [details]

Comment 3

[Ed Lee :Mardak]

Another screenshot from bug 991804 attachment 8401435 [details].

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

I think the box-shadow on the panel may need to be set to 'none'.

Comment 5

[Ed Lee :Mardak]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #4)
> I think the box-shadow on the panel may need to be set to 'none'.
Thanks. emtwo, can you take a look on your windows machine? I hope it's as simple as just opening a tab, clicking the panel and verifying in inspector that there's a box-shadow from somewhere.

Comment 6

[Marina Samuel [:emtwo]]

I've explored this a bit but didn't make much progress.

I've tried setting box-shadow to none on #sponsored-panel as well as making changes to toolkit/themes/windows/global/popup.css such as panel's border, margin, color. Any suggestions where else this could be coming from or how to debug it? Thanks! :jaws, :Mardak

Or maybe can pass this bug on to someone more familiar with the arrow panel? I don't think this bug is sponsored-panel specific as far as I can tell.

Comment 7

[Ed Lee :Mardak]

enn/dao, have either of you run into this odd shadowing issue when implementing the animation?

emtwo, can you try applying bug 610545 to see if something magically gets fixed?

Comment 8

[Dão Gottwald [:dao]]

Is this a standard arrow panel or something custom?

Comment 9

[Marina Samuel [:emtwo]]

http://dxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.xul#20 It's a panel with type arrow, not a custom binding

Comment 10

[Marina Samuel [:emtwo]]

Applying bug 610545 didn't fix the issue... but the animation is pretty cool!

Comment 11

[Marina Samuel [:emtwo]]

Hey Neil, do you have any insight on this issue? :jaws and :mconley have looked without much progress as well and suspect it's something lower level. Thanks!

Comment 12

[Neil Deakin]

We don't allow translucent windows in non-chrome docshells. We should probably change this to be based on the popup being in a privileged page or somesuch.

Comment 13

[Neil Deakin]

OK, that's not quite correct. It's just the windows dropshadow (CS_DROPSHADOW) that is blocked in content shells. Actually there are two options, and we could do either or both:

1. Do what I suggested above. That would also fix the popup so that it wasn't constrained by the window border.
2. Just remove the check content shell check since you can just make transparent popups on other platforms anyway.

Comment 14

[Neil Deakin]

Attachment: Allow drop shadows in content shell popups

This patch implements option 2.

Transparent windows have been allowed since bug 451300 (although I think the patch for that bug wasn't correct).

Comment 15

[Marina Samuel [:emtwo]]

Comment on attachment 8403499 [details] [diff] [review]
Allow drop shadows in content shell popups

Thanks very much for looking into this Neil and adding the patch! I verified that this fixes the issue on windows and doesn't seem to mess with anything on mac. I'm adding :tn for review here.

Comment 16

[Timothy Nikkel (:tnikkel)]

(In reply to Neil Deakin from comment #14)
> Transparent windows have been allowed since bug 451300 (although I think the
> patch for that bug wasn't correct).

Don't we need xul to create transparent panels? And xul is not allowed in typical unprivileged content. So perhaps this got fixed by disabling xul?

Comment 17

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8121fe54033

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a8121fe54033

Comment 19

[u279076]

Florin, please assign this for verification in this iteration. I know it's late in the iteration so please needinfo me if testing cannot be completed before the weekend.

Comment 20

[Cornel Ionce [:noni] [Hubs QA]]

I'll work on this issue and update the results later today.

Comment 21

[Cornel Ionce [:noni] [Hubs QA]]

I cannot verify this fix because the grid is no longer populated. For a clean profile, the newtab page is only having the first tile populated (First Run page) and the rest of them are empty.

It seems this started from yesterday's nightly (2014-04-10).

Any ideas what could have happened?

Comment 22

[Ed Lee :Mardak]

(In reply to Cornel Ionce [QA] from comment #21)
> I cannot verify this fix because the grid is no longer populated.
The list was temporarily removed for now, you can see it again by changing

browser.newtabpage.directorySource pref to..
chrome://global/content/directoryLinks.json

Comment 23

[Cornel Ionce [:noni] [Hubs QA]]

Thanks for your response.

By changing the pref mentioned in comment 22 I was able to confirm this fix using latest Nightly (build ID: 20140411030201) on Windows 7 and Windows 8.1