2.35 KB, application/x-xpinstall
5.59 KB, patch
Mats Palmgren (vacation - back in August): review+
|Details | Diff | Splinter Review|
5.36 KB, patch
|Details | Diff | Splinter Review|
7.74 KB, image/jpeg
Created attachment 672129 [details] XPI testcase Steps to reproduce: (1) Install the attached extension in a Firefox profile (2) There will be a new toolbar in Firefox with an "Open dialog!" button. (3) Click on the button. Expected results: A dialog box appears with the words "powered by example.com" and a button below it. Actual results: Dialog box appears, but its contents are all black.
My colleagues at APN discovered this in testing our company's extension in Firefox 17 beta. This testcase is near-minimal: almost any reduction of it leads to the black box disappearing.
The issue appears in FF18 firstly then has been backported into FF16/17. If I disable HWA, the dialog box contents is invisible but the background stays gray (default). With HWA enabled, background is black. In Firefox 16, the dialox box is "malformed" and the OK button can't be pressed: http://i.imgur.com/uc5fV.jpg m-c good=2012-09-17 bad=2012-09-18 http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3f0587ce1774&tochange=0d3b17a88d5f My guess goes to this suspected bug: Robert O'Callahan — Bug 788877. Don't create nsDisplayFixedPosition items when the root scroll frame is inactive. r=Cwiiis,a=akeybl
For the record, the changelog for m-a repo: http://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=7676a9a06403&tochange=006174be2306
(In reply to Alex Vincent [:WeirdAl] from comment #1) > My colleagues at APN discovered this in testing our company's extension in > Firefox 17 beta. This testcase is near-minimal: almost any reduction of it > leads to the black box disappearing. Can you confirm that the dialog box appears as desired in FF16? I see you marked that version as unaffected, but the suspected bug 788877 is in that version as well.
(In reply to Alex Keybl [:akeybl] from comment #4) > Can you confirm that the dialog box appears as desired in FF16? I see you > marked that version as unaffected, but the suspected bug 788877 is in that > version as well. No, the dialog appears as Loic indicates in comment 2 - I don't have it in front of me, but I remember it clearly. I was initially concerned only with the rendering being a black box; I didn't think to test whether the button was clickable or not.
We hit a lot of NS_ERROR("D3D10 should never need to update ThebesLayers in an empty transaction"); I'm pretty sure that's the problem.
The dialog box looks like this: <window class="dialog" id="hello" title="Hello" xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <vbox> <text id="hello-label" value="powered by example.com" style="padding: 16px;" /> </vbox> <hbox style="position: fixed; right: 16px; bottom: 16px;"> <button label="OK" /> </hbox> </window> It looks to me like the button *should* overlap the label, as shown in the screenshot in comment #2. You should be able to click on it though. The problem here actually seems to be a layout problem. The root frame for the dialog has [vis-overflow=0,0,13320,1073740864], which is very bad and is causing rendering/layers to blow up.
The fixed-pos hbox is positioned at y=1073738764.
Mixing right/bottom positioning with intrinsic window sizing is the problem here, I think.
FWIW, the person who wrote the original code that my testcase derives from did not understand XUL - but since it was never a problem, we never even noticed... we can definitely fix our code to use XUL spacers and position things correctly, but it could take a little while (lots of little fires to put out).
Created attachment 672668 [details] [diff] [review] fix
Comment on attachment 672668 [details] [diff] [review] fix r=mats
Note: The fix does not apply cleanly to FF 17 Beta branch.
(In reply to Alex Vincent [:WeirdAl] from comment #13) > Note: The fix does not apply cleanly to FF 17 Beta branch. Please nominate a patch that does apply cleanly for uplift to branches. If we can get this in before EOD PT Monday (tomorrow) we can ship this fix in 17.0 Beta 3 and get testing on it with time to catch regressions if any occur.
Created attachment 674090 [details] [diff] [review] beta patch [Approval Request Comment] Bug caused by (feature/regressing bug #): possibly regressed by bug 788877, but it's unclear; in any case, it's probably an old bug that we just uncovered recently User impact if declined: Unusual dialogs in some extensions will not render Testing completed (on m-c, etc.): none really Risk to taking this patch (and alternatives if risky): pretty low risk. Only affects intrinsic-sized windows (quite rare --- only XUL dialog boxes, really, so not Web content), and position:fixed is probably quite rare in those. No real alternative to taking the patch. String or UUID changes made by this patch: none.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16) > Testing completed (on m-c, etc.): none really It's on nightly now, so adding qawanted to verify before approving uplift.
I can confirm that the patch for beta fixes the original issue we spotted this for. Thanks!
Comment on attachment 674090 [details] [diff] [review] beta patch Thanks, will remove qawanted, approving for uplift.
Created attachment 683166 [details] dialog Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 RC The dialog box contains "powered by" text and an OK button which doesn't do anything when clicked. (see attached screenshot) Did the functionality of the addon change?