Closed
Bug 425844
Opened 17 years ago
Closed 16 years ago
[10.5] Print/Page Setup Menu items grayed out after printing a page using File | Print
Categories
(Core :: Widget: Cocoa, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: marcia, Assigned: jtd)
References
Details
(Keywords: regression, verified1.9.0.7, verified1.9.1, Whiteboard: [See comments #26 and #37])
Attachments
(2 files, 5 obsolete files)
(deleted),
patch
|
jaas
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jaas
:
review+
dveditz
:
approval1.9.0.7+
|
Details | Diff | Splinter Review |
Seen using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008032804 Minefield/3.0pre.
STR:
1. Go to a web page and select File | Print | Save as PDF
2. Go back to the File menu and inspect the file choices.
Expected: All would be active
Actual: New Tab, Close, Save Page As, Send Link, Page Setup and Print are greyed out. I can get the menu items back easily by switching focus off the page.
This does not happen using the equivalent nightly on Tiger.
Reporter | ||
Comment 1•17 years ago
|
||
I actually just noticed when testing on PPC mac that many other menus are greyed out besides the File Menu after following the STR in Comment 0. Switching focus out of the window and back restores all the menu items to active.
Comment 2•17 years ago
|
||
That's a regression which has been caused by a fix with the last days. Running a test with beta5 rc2 doesn't show this problem. I'll try to get the regression window.
Asking for blocking-firefox3 because of limited functionality.
Comment 3•17 years ago
|
||
Regression window: 20080327-04 and 20080328-04
Checkins in this timeframe:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-03-27+03%3A00%3A00&maxdate=2008-03-28+04%3A00%3A00&cvsroot=%2Fcvsroot
Possibly related: bug 425593
Comment 4•17 years ago
|
||
I'm not sure I've understood your description, Marcia. But as far as
I can tell this isn't a bug.
The Save As dialog you get when you do File | Print | Save as PDF is a
native modal dialog (one provided by the OS, and not by Gecko). While
it's running, it correctly disables a number of menu items, including
the ones you mention. The idea is that this is a modal dialog, and
you need to finish whatever tasks it requires before you go off to do
anything else. It works the same way in Safari.
When the dialog closes (whether you press Save or Cancel) the
greyed-out menu items always get re-enabled (at least in my tests).
Comment 5•17 years ago
|
||
(In reply to comment #4)
> When the dialog closes (whether you press Save or Cancel) the
> greyed-out menu items always get re-enabled (at least in my tests).
Exactly this doesn't happen under OS X 10.5. After you close the print dialog these menu items are not re-enabled. You have to switch the focus (like Marcia said) to get the menu items re-enabled.
Comment 6•17 years ago
|
||
> After you close the print dialog these menu items are not re-enabled.
I can't reproduce this. I tested with today's nightly on OS X 10.5.2 (Intel).
Comment 7•17 years ago
|
||
> I can't reproduce this.
Sorry, I take that back. I _can_ reproduce it..
Comment 8•17 years ago
|
||
My patch for bug 422827 landed in Henrik's regression range from comment #3, and I was a _little_ afraid ... but I've now exonerated it.
Disabling my patch for bug 422827 doesn't fix this bug -- which shows that it's unrelated.
Comment 9•17 years ago
|
||
> Possibly related: bug 425593
Nope. Backing out that bug's patch doesn't fix this problem.
Comment 10•17 years ago
|
||
Steven, are you able to verify the regression range? If not, I was wrong. Otherwise another patch listed by the above query should be responsible for.
Comment 11•17 years ago
|
||
> Steven, are you able to verify the regression range?
I haven't had a chance to (I haven't tried yet). I hope to do so tonight or tomorrow.
Comment 12•17 years ago
|
||
Within the regression range, bug 425593 is probably covering up (hiding) the issue.
Starting with this hourly build: 20080326_1830, the print dialogue throws up a sheet (bug 425593). Clicking in the sheet to dismiss it shifts the focus around (somehow the steps in comment 1), and restores the functionality of the menus
I would be more suspicious of bug 372571 (that also causes 'vanishing menu issues' in Camino, see bug 426011). Bug 372571 landed slightly earlier than the regression range.
And with the 20080326_1732 hourly build, I can't reproduce the issue.
Comment 13•17 years ago
|
||
Philippe, where do you get these hourly builds?
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1
Comment 14•17 years ago
|
||
(In reply to comment #13)
> Philippe, where do you get these hourly builds?
>
http://hourly-archive.localgho.st/mac.html
Comment 16•17 years ago
|
||
(In reply to comment #12)
> Within the regression range, bug 425593 is probably covering up (hiding) the
> issue.
Not really, see bug 426990. I experienced the exact same thing, and it's because an OS window is taking focus. Oddly, the upload dialog doesn't do it, but whenever I upgrade my nightly build and have to reauthorize access to the 1passwd keychain for the changed Minefield.app, coming back to Minefield breaks New Tab (etc). I've duped my bug here because I'm certain it's the same, it's accessing an OS X feature and doesn't really recognize that focus has returned. That's why (I think) switching apps back and forth fixes the issue.
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008040404 Minefield/3.0pre
Comment 17•17 years ago
|
||
This bug, whatever might have triggered it, is the result of our Carbon printing dialog doing whatever it wants to our Cocoa menus. We can't override that in the obvious way like we can for Cocoa app-modal dialogs. We need to get rid of our Carbon printing dialog and replace it with a Cocoa one. That said, there may be a way to work around this, which we can do for Gecko 1.9.0.x.
This bug is much less severe now that the patch for bug 398514 has landed. Now the keyboard commands will work just fine even in the menu items are disabled.
Flags: wanted1.9.0.x+
Flags: blocking1.9-
Flags: blocking1.9+
Priority: P1 → P2
Comment 18•17 years ago
|
||
@Josh Aas
That makes sense for printing, but does that mean that bug 426990 should be unmarked as a dupe since it's a separate bug with the same effects?
Comment 19•17 years ago
|
||
I'd leave them both open for now, until we dig into more gritty details.
Reporter | ||
Comment 20•17 years ago
|
||
Just as a point of info it happens when not only with Print to PDF, but print too so I am updating the summary of this bug.
Keywords: relnote
Summary: [10.5] Menu items grayed out when Print | Save as PDF is selected → [10.5] Menu items grayed out after printing a page using File | Print
Comment 21•16 years ago
|
||
Bug 433778 is probably related ... though it isn't a dup.
Comment 22•16 years ago
|
||
(In reply to comment #21)
> Bug 433778 is probably related ... though it isn't a dup.
Perhaps Masayuki can help here...
Comment 24•16 years ago
|
||
OS X 10.5.3 here, with FF3RC2, and when I click Print, then Cancel, I have to either create a new Firefox window or switch to another application, or open a Firefox sub-window (preferences, open a file, etc). Switching tabs doesn't do anything.
Comment 25•16 years ago
|
||
(In reply to comment #24)
> OS X 10.5.3 here, with FF3RC2, and when I click Print, then Cancel, I have to
> either create a new Firefox window or switch to another application, or open a
> Firefox sub-window (preferences, open a file, etc). Switching tabs doesn't do
> anything.
>
Switching tabs doesn't because all existing tabs have the bug. Try it again, but this time after hitting cancel, do Command-T to open a new tab. I think that, for some reason, re-enabled the menu items (at least for me).
Reporter | ||
Comment 26•16 years ago
|
||
Another way to get the menu item re-enabled is to switch focus outside the Firefox window. When you focus back on the app you will again have the "print" menu available. Just a workaround until we get this bug fixed...
Comment 27•16 years ago
|
||
Just for the record, this bug is still here in FF3RC3 on OSX 10.5.3 (Intel).
It "simply" looks like menus are not reset after the Print dialog is closed. If you open the Print dialog and look at the menus while the focus is on it, you will see that many menu items are grayed out, which is probably correct, as you cannot do any browsing-related operation on the Print dialog. But the problem is that these grayed out menus stay the same even after closing the Print dialog somehow (hitting Cancel, printing, etc.).
Updated•16 years ago
|
Flags: blocking1.9.1?
Comment 34•16 years ago
|
||
We also see this bug. Mac OS X 10.5.3 on PPC using 3.0.
Comment 36•16 years ago
|
||
I am seeing this same problem on 10.5.4. Going to Page Setup will then gray out the Page Setup and Print options in the File menu.
Comment 37•16 years ago
|
||
Please do not comment that you still see this bug. It is known to exist and with the large amount of duplicates, it is confirmed that many people are seeing it. It is confirmed. Commenting that you still see it e-mails all 'subscribed' to this bug with 'bugspam'. It will not cause the bug to become fixed any sooner. Patches, however, are welcome.
Whiteboard: [See comments #26 and #37]
Comment 44•16 years ago
|
||
As one of the people who reported this, I am more interested in understanding when we will see a version that has the fix for it. When is a new release due? RJ
Comment 45•16 years ago
|
||
You won't see a version that has a fix for it until at least one day after someone has submitted a patch for it.
Comment 46•16 years ago
|
||
Is anyone working on a patch for it?
How do I get access to a patch when it is available?
Comment 47•16 years ago
|
||
(In reply to comment #46)
> Is anyone working on a patch for it?
>
> How do I get access to a patch when it is available?
Yes, and just follow bug 456646 and wait until it has been marked fixed. That means that the patch has been pushed into the repository and nightlies are being built from it. Please refrain from needlessly commenting on the bug, as this creates extra bugmail for everyone on the CC list. Thanks.
Comment 48•16 years ago
|
||
> Please refrain from needlessly commenting on the bug, as this creates extra bugmail for everyone on the CC list.
This wasn't "needless commenting", it was informative followup. Thanks RJS for your questions and Thomas for your answers. As someone on the CC list I'm glad there has been some progress.
Ryan
Comment 49•16 years ago
|
||
(In reply to comment #48)
> This wasn't "needless commenting", it was informative followup.
Ryan, I was referring to bug 456646, because he said:
>I split this work into a new bug to get a better tracking.
He obviously created the new bug to make it easier to resolve, as the only comments should be ones related to the patch and not related to a new dupe or questions about when the bug will be resolved.
Sorry for the confusion.
Comment 51•16 years ago
|
||
Have tracked through the 50 comments. (My comments were no.s 44 and 46.)
I am not familiar with the protocols re bug fixes, as I am just a user of Firefox, not a developer.
Is anyone working on the fix? Have looked at 456646 and there seems no progress there.
When this item is fixed where do I go to get an updated version?
Thanks
RJ
Comment 52•16 years ago
|
||
(In reply to comment #51)
> Have tracked through the 50 comments. (My comments were no.s 44 and 46.)
>
> I am not familiar with the protocols re bug fixes, as I am just a user of
> Firefox, not a developer.
>
> Is anyone working on the fix? Have looked at 456646 and there seems no progress
> there.
your answer is partly in comment 45 and 47 - they get fixed when someone has the time and the insight to code the solution. It could be hours, it could be weeks - asking when generally doesn't improve the odds of it happening sooner.
> When this item is fixed where do I go to get an updated version?
when the patch lands, the next day it will be in ftp://ftp.mozilla.org/pub/firefox/nightly/latest-trunk/
Updated•16 years ago
|
Flags: wanted1.9.1?
Comment 57•16 years ago
|
||
Lots of items marked as duplicates - the duplicates showing many people are seeing the problem. But no one has done anything to fix it. (I would, but have no technical skills.)
Is there a work around?
Comment 58•16 years ago
|
||
> Is there a work around?
Sure, don't use the menu to print: cmd+p
Comment 59•16 years ago
|
||
(In reply to comment #58)
> > Is there a work around?
>
> Sure, don't use the menu to print: cmd+p
Or see the workaround in comment 26, mentioned in the whiteboard, and the request not to say "I still see this and nobody's posted a workaround" in comment 37, also mentioned in the whiteboard.
Reporter | ||
Updated•16 years ago
|
Summary: [10.5] Menu items grayed out after printing a page using File | Print → [10.5] Print/Page Setup Menu items grayed out after printing a page using File | Print
Assignee | ||
Comment 62•16 years ago
|
||
This bug is actually a lot more serious than would appear at first glance. There are actually two parts to this bug: (1) while the Page Setup or Print dialogs are up, many menu items are enabled which shouldn't be and (2) once the dialog is dismissed, the state of menus remains in the the same state it was in while those dialogs were up. The first part is cosmetic, nothing bad happens, but the second is why so many folks have logged dupes on this, because it hinders functionality.
So it's not just some items in the File menu, but *most* menu items are out of sync:
File >
Close enable ==> disable
Save Page As enable ==> disable
Send Link enable ==> disable
Page Setup enable ==> disable
Print enable ==> disable
Edit >
Undo disable ==> enable (WTF?!?)
Redo disable ==> enable
Cut disable ==> enable
Paste disable ==> enable
Delete disable ==> enable
Find disable ==> enable
Find Again disable ==> enable
View >
*all* enable ==> disable
Bookmarks >
Bookmark This Page enable ==> disable
Bookmark All Tabs enable ==> disable
Tools >
Page Info enable ==> disable
Window >
Minimize enable ==> disable
Zoom enable ==> disable
We currently call the Carbon printing API's PMSessionPageSetupDialog and PMSessionPrintDialog, which is a bit odd in a Cocoa app. So I'm sure the right approach is to move to Cocoa printing API's as Josh suggests. However that's a big, out-of-scope task for either 3.1 or a 3.0 hotfix. So I would suggest this, that we call a simple utility method that iterates over menu bar items and assures that they are in sync with the content, similar to what's going on here:
http://mxr.mozilla.org/mozilla-central/source/widget/src/cocoa/nsMenuItemX.mm#357
Call this method directly or from an event here:
http://mxr.mozilla.org/mozilla-central/source/embedding/components/printingui/src/mac/nsPrintingPromptServiceX.mm#349
and here:
http://mxr.mozilla.org/mozilla-central/source/embedding/components/printingui/src/mac/nsPrintingPromptServiceX.mm#487
I really, really think this needs to block for 1.9.1.
Screenshot of menus before and after bringing up the Page Setup dialog (before = top, after = bottom):
http://people.mozilla.org/~jdaggett/menusbeforeafter.png
Steps to reproduce:
1. Go to any page with text
2. Select a word
3. File > Page Setup
4. Click Cancel
Result: Menus change state from before to after as in the screenshot
Expected result: Menus should return to their original state
Comment 63•16 years ago
|
||
John, please also see bug 456646, which handles the switch from Carbon to Cocoa.
Comment 64•16 years ago
|
||
Re-nominating based on the workaround proposal in comment 62.
Flags: blocking1.9.1- → blocking1.9.1?
Reporter | ||
Comment 65•16 years ago
|
||
I agree with Comment 62. In my daily work, this is probably the most annoying Mac bug I see consistently and the one most often mentioned by Hendrix mac users.
Assignee | ||
Updated•16 years ago
|
Assignee: joshmoz → jdaggett
Assignee | ||
Comment 66•16 years ago
|
||
I tracked down the change that caused this problem, it was the patch checked in to fix bug 372571, 2008-03-26 20:42 (josh).
To reproduce this, pull CVS trunk with MOZ_CO_DATE="26 Mar 2008 20:35 PDT". Build and the problem does not occur. Apply the patch for 372571 and it occurs.
This jives with the appearance of this bug in nightly builds, I came up with a slightly different regression window than the one suggested in comment 3 (and Philippe suggested it might be this bug in comment 12):
Bug does not occur: 2008-03-26-04 (4:47 AM)
Bug occurs: 2008-03-27-04 (4:49 AM)
Code changes during this time period:
http://tinyurl.com/macmenubugchanges
Assignee | ||
Comment 67•16 years ago
|
||
I don't know why this works but taking the two new routines from bug 372571, PrepareForNativeAppModalDialog and CleanUpAfterNativeAppModalDialog in nsCocoaUtils, and bracketing the calls to Carbon print dialog routines seems to eliminate the problem. I stuck access methods for these in nsPrintSettingsX because I'm not quite sure how to call widget code from within the printgui component. Lameness supreme...
Josh and/or Steven, can you take a look and let me know what you think the correct way of fixing this is?
Attachment #358791 -
Flags: review?(joshmoz)
Comment 68•16 years ago
|
||
Comment on attachment 358791 [details] [diff] [review]
utterly lame patch, v.0.1, bracket calls to print dialog code with menu fixup
>+++ b/widget/public/nsIPrintSettingsX.idl
You should update the uuid
..and remove printfs ;)
Comment 69•16 years ago
|
||
Comment on attachment 358791 [details] [diff] [review]
utterly lame patch, v.0.1, bracket calls to print dialog code with menu fixup
I tried something like this a while ago and it had no effect - the menu preparation did nothing for Carbon dialogs. I may have missed a spot or made some other mistake though.
Please remove the debugging printfs and rev the uuid like smaug said.
+ /* terribly rude hack */
Please remove this comment. This is how things are supposed to work given the way XUL and Mac OS X interact.
+ [noscript] void PrepareForNativeAppModalDialog();
+ [noscript] void CleanUpAfterNativeAppModalDialog();
The first character of method names should not be capitalized in idl files. See methods above the ones you added.
Once that is taken care of, if the patch works in your testing I'm fine with the code. This needs to be tested on Mac OS X 10.4 and 10.5 before landing - if you don't have one of those versions to test on let us know, smichaud can probably help you out.
Comment 70•16 years ago
|
||
blocking1.9.1+ so long as the approach in John Daggett's patch works. With a workable strategy we should not ship with this bug. However, if John's patch doesn't work out and we get back to rewriting our print dialogs in Cocoa we're going to have to minus this bug since that changes is unacceptable for 1.9.1 at this point.
Flags: wanted1.9.1?
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Attachment #358791 -
Flags: review?(joshmoz) → review-
Assignee | ||
Comment 71•16 years ago
|
||
This is not strictly dependent on Cocoa printing dialogs, bug 456646, removing the dependency.
No longer depends on: 456646
Assignee | ||
Comment 72•16 years ago
|
||
Cleaned up version of the original patch. It would be nice to have a better place to put this but leaving it as is for now.
Tested on 10.5, the graying out behavior is gone. Although the code uses code simiilar to the code used to display an Edit menu when opening a file, this menu doesn't appear. So Edit menu behavior remains broken, same as before.
Attachment #358791 -
Attachment is obsolete: true
Attachment #358993 -
Flags: review?(joshmoz)
Assignee | ||
Comment 73•16 years ago
|
||
Tested on 10.4. Before the patch when selecting Page Setup/Print, the entire menu bar would appear but edit controls (cut/copy/paste) would not work. With this patch a single Edit menu appears and edit controls are usable to some degree but not all. So the behavior is better, but there are still problems with edit controls that need to get fixed. But that's a much smaller problem than menu items being out of whack on 10.5.
Try server build available here for those who want to test:
http://tinyurl.com/fixmenudisable
Comment 74•16 years ago
|
||
John, I've tested the tryserver build and it looks great on 10.5. Everything is accessible after the the print dialog was open. All menus look fine.
Comment 75•16 years ago
|
||
I have tested the tryserver build with 10.5. (10.5.6) as well and had some strange hangs with Print and Save Page, which were not reproducible. But this one is reproducible now. Before was running FF 3.0.5. Start build, open a webpage and try Save Page as ...
Build hangs after some sec. and uses 99% Cpu.
Comment 76•16 years ago
|
||
John, after this patch lands on m-c and 1.9.1, can you please work up a 1.9.0 patch?
Assignee | ||
Comment 77•16 years ago
|
||
(In reply to comment #75)
> I have tested the tryserver build with 10.5. (10.5.6) as well and had some
> strange hangs with Print and Save Page, which were not reproducible. But this
> one is reproducible now. Before was running FF 3.0.5. Start build, open a
> webpage and try Save Page as ...
> Build hangs after some sec. and uses 99% Cpu.
This sounds like a separate bug. Do you open the Save Page As dialog without choosing either Page Setup or Print first? If so, this patch isn't the problem, this affects 3.1 beta builds also most likely.
Are these the steps you're using?
1. Start tryserver build
2. Open webpage
3. File > Save Page As
Result: build hangs and CPU goes to 99%
Is this what you're seeing? (I'm on 10.4 right now, will test on 10.5 a little later).
Assignee | ||
Comment 78•16 years ago
|
||
(In reply to comment #76)
> John, after this patch lands on m-c and 1.9.1, can you please work up a 1.9.0
> patch?
will do.
Assignee | ||
Comment 79•16 years ago
|
||
Testing on 10.5, can't reproduce the behavior noted in comment 72.
No longer blocks: 475483
Assignee | ||
Comment 80•16 years ago
|
||
I think this is a much simpler version of the fix, namely to simply repaint the menu bar of the main window after calling either the page setup or print dialogs:
NSWindow* mainWindow = [NSApp mainWindow];
if (mainWindow)
[WindowDelegate paintMenubarForWindow:mainWindow];
This fixes the menu state problem on 10.5 and leaves 10.4 behavior completely unaffected. It's effectively the equivalent of the workarounds mentioned, to switch to another window and then back again to restore the menu state.
Attachment #358993 -
Attachment is obsolete: true
Attachment #359175 -
Flags: review?(joshmoz)
Attachment #358993 -
Flags: review?(joshmoz)
Comment 81•16 years ago
|
||
(In reply to comment #77)
It's true, John, it is a seperate bug. I checked this bug and it appears only if there are more than two files to store in sub folder. It doesn't matter if these are include files (js, css...) or images.
There is no bug reported at this time, I guess. Could be this a bug only in this build? And/or under which version it should be reported?
Assignee | ||
Comment 82•16 years ago
|
||
Yeah, my guess is you've found a bug in 3.1/3.2 code that's not in 3.0. The try server build is equivalent to using the latest nightly build, so try your test with a latest trunk nightly:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
If you run into a problem with one of these builds, log a bug with version "Trunk" and include the build number (<app> menu > About Minefield). Be sure to include steps to reproduce, sounds like you're seeing a problem when saving a specific type of page, right?
Assignee | ||
Comment 83•16 years ago
|
||
Try server build with latest patch for those who want to test:
http://tinyurl.com/fixmenudisablev03
Comment 84•16 years ago
|
||
(In reply to comment #82)
> Yeah, my guess is you've found a bug in 3.1/3.2 code that's not in 3.0. The
> try server build is equivalent to using the latest nightly build, so try your
> test with a latest trunk nightly:
>
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
>
> If you run into a problem with one of these builds, log a bug with version
> "Trunk" and include the build number (<app> menu > About Minefield). Be sure
> to include steps to reproduce, sounds like you're seeing a problem when saving
> a specific type of page, right?
it is a known issue - and already fixed: bug 475078.
Attachment #359175 -
Flags: review?(joshmoz) → review-
Comment 85•16 years ago
|
||
Comment on attachment 359175 [details] [diff] [review]
patch, v.0.3, simpler version, just paint main window menu bar
>+ * After bringing up Carbon print dialogs, clean up menus.
>+ */
>+ [noscript] void cleanUpAfterDialog();
>+
Lets call this "cleanUpAfterCarbonDialog" to make it clear when this is to be used. Please note a bug number in the comment for this new method. Get rid of the extra newline added after the method.
>+NS_IMETHODIMP nsPrintSettingsX::CleanUpAfterDialog()
>+{
>+ NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT;
>+
>+ NSWindow* mainWindow = [NSApp mainWindow];
>+ if (mainWindow)
>+ [WindowDelegate paintMenubarForWindow:mainWindow];
>+
>+ NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(noErr);
>+}
The exception code here is returning "noErr", a Carbon return value for no error, but the method signature says it returns an nsresult value. Change "NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(noErr)" to "NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT".
Also, we probably want to paint the menu bar for the hidden window if there is no main window after the dialog closes. See the logic for that in "nsCocoaUtils::CleanUpAfterNativeAppModalDialog()". The hidden window is the window that we use when there appears to be no windows open in a Gecko application.
Assignee | ||
Comment 86•16 years ago
|
||
Attachment #359175 -
Attachment is obsolete: true
Attachment #359423 -
Flags: review?(joshmoz)
Attachment #359423 -
Flags: review?(joshmoz) → review+
Assignee | ||
Comment 87•16 years ago
|
||
Assignee | ||
Comment 88•16 years ago
|
||
1.9.0 patch, as per comment 76. die bug die
Attachment #359487 -
Flags: approval1.9.0.7?
Assignee | ||
Comment 89•16 years ago
|
||
adjusted to changed location for hidden menu routine
Attachment #359487 -
Attachment is obsolete: true
Attachment #359495 -
Flags: approval1.9.0.7?
Attachment #359487 -
Flags: approval1.9.0.7?
Comment 90•16 years ago
|
||
John-san, you must not change any interfaces on stable branch. You need to create a new interface for the new APIs.
See following patch:
https://bugzilla.mozilla.org/attachment.cgi?id=217349&action=diff
(In reply to comment #90)
> John-san, you must not change any interfaces on stable branch. You need to
> create a new interface for the new APIs.
> See following patch:
> https://bugzilla.mozilla.org/attachment.cgi?id=217349&action=diff
It should be fine to change the stable interface provided the UUID is revved and/or that the new function is added to the end. I think both things are being done here, so it should be fine.
Comment 92•16 years ago
|
||
Thanks John! I think we can remove relnote now.
Verified with:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090129 Minefield/3.2a1pre ID:20090129020336
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090129 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090129021345
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla1.9.2a1
Comment 93•16 years ago
|
||
(In reply to comment #91)
> (In reply to comment #90)
> > John-san, you must not change any interfaces on stable branch. You need to
> > create a new interface for the new APIs.
> > See following patch:
> > https://bugzilla.mozilla.org/attachment.cgi?id=217349&action=diff
>
> It should be fine to change the stable interface provided the UUID is revved
> and/or that the new function is added to the end. I think both things are
> being done here, so it should be fine.
Really? I was not allowed in the bug (see bug 187772 comment 31).
In that particular case, it's likely because nsIRollupListener was added to the concrete implementation class' inheritance tree -- which may or may not have changed the concrete class layout. I think it still should have been fine though, as people should have been QI'ing anyway to get the other implementations; mconnor was just being overzealous, I guess.
But, I don't know for sure -- would need a call from the 1.9.0 drivers, though. To me, it would be fine to just add it at the end.
Comment 95•16 years ago
|
||
We have never allowed interface changes on stable branches under any circumstances.
Assignee | ||
Comment 96•16 years ago
|
||
Same patch but moved into a separate interface for 1.9.0.x branch code.
Attachment #359495 -
Attachment is obsolete: true
Attachment #360034 -
Flags: review?(joshmoz)
Attachment #360034 -
Flags: approval1.9.0.7?
Attachment #359495 -
Flags: approval1.9.0.7?
Updated•16 years ago
|
Whiteboard: [See comments #26 and #37] → [needs r=josh][See comments #26 and #37]
Comment 97•16 years ago
|
||
nsIPrintSettingsX_MOZILLA_1_9_BRANCH should be inherited nsIPrintSettingsX. Then:
-NS_IMPL_ISUPPORTS_INHERITED1(nsPrintSettingsX,
+NS_IMPL_ISUPPORTS_INHERITED2(nsPrintSettingsX,
nsPrintSettings,
- nsIPrintSettingsX)
+ nsIPrintSettingsX,
+ nsIPrintSettingsX_MOZILLA_1_9_BRANCH)
can be:
NS_IMPL_ISUPPORTS_INHERITED1(nsPrintSettingsX,
nsPrintSettings,
- nsIPrintSettingsX)
+ nsIPrintSettingsX_MOZILLA_1_9_BRANCH)
And when a client gets nsIPrintSettingsX_MOZILLA_1_9_BRANCH interface, it can also use nsIPrintSettingsX interface. So, you can reduce to query interface in nsPrintingPromptServiceX.mm.
Comment 98•16 years ago
|
||
Comment on attachment 360034 [details] [diff] [review]
patch, v.0.4c, create 1.9.0 branch version
Masayuki is probably right, waiting for a response from John.
Comment 99•16 years ago
|
||
Comment on attachment 360034 [details] [diff] [review]
patch, v.0.4c, create 1.9.0 branch version
I'm actually fine either way, though please try what Masayuki suggested. The rest of this looks fine to me.
Attachment #360034 -
Flags: review?(joshmoz) → review+
Comment 100•16 years ago
|
||
(In reply to comment #97)
> nsIPrintSettingsX_MOZILLA_1_9_BRANCH should be inherited nsIPrintSettingsX.
Could or should, right.
> Then:
>
> -NS_IMPL_ISUPPORTS_INHERITED1(nsPrintSettingsX,
> +NS_IMPL_ISUPPORTS_INHERITED2(nsPrintSettingsX,
> nsPrintSettings,
> - nsIPrintSettingsX)
> + nsIPrintSettingsX,
> + nsIPrintSettingsX_MOZILLA_1_9_BRANCH)
>
> can be:
>
> NS_IMPL_ISUPPORTS_INHERITED1(nsPrintSettingsX,
> nsPrintSettings,
> - nsIPrintSettingsX)
> + nsIPrintSettingsX_MOZILLA_1_9_BRANCH)
This is not true. This would mean the the object doesn't support nsIPrintSettingsX, but it supports only nsIPrintSettingsX_MOZILLA_1_9_BRANCH.
QueryInterfacing to nsIPrintSettingsX would fail.
Comment 101•16 years ago
|
||
Comment on attachment 360034 [details] [diff] [review]
patch, v.0.4c, create 1.9.0 branch version
Approved for 1.9.0.7, a=dveditz for release-drivers.
>+interface nsIPrintSettingsX_MOZILLA_1_9_BRANCH : nsISupports
subclassing nsIPrintSettingsX instead of nsISupports would have accomplished what comment 97 was getting at. Olli's right about the macros, though.
Attachment #360034 -
Flags: approval1.9.0.7? → approval1.9.0.7+
Assignee | ||
Comment 102•16 years ago
|
||
I'm going to leave this as-is. I can definitely see the reason for subclassing vs. using a mix-in class but this is a very simple piece of code on a branch, I don't see any compelling reason to add more complexity in the form of subclasses to the problem.
Keywords: fixed1.9.0.7
Assignee | ||
Comment 103•16 years ago
|
||
Checked in to CVS trunk
Comment 104•16 years ago
|
||
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009020404 GranParadiso/3.0.7pre ID:2009020404
Keywords: fixed1.9.0.7 → verified1.9.0.7
Comment 105•16 years ago
|
||
(In reply to comment #101)
> subclassing nsIPrintSettingsX instead of nsISupports would have accomplished
> what comment 97 was getting at. Olli's right about the macros, though.
Oh, right. I didn't do so in bug 187772, sorry.
Updated•16 years ago
|
Whiteboard: [needs r=josh][See comments #26 and #37] → [See comments #26 and #37]
Comment 107•16 years ago
|
||
Marcia, as you have said this is not fixed for 10.6. Is this still true?
Comment 108•16 years ago
|
||
Firefox is currently the only application on MacOSX Intel 10.5.6 that have this bug.
Not be able to print in 2009 is very poor and we will switch back to Safari and forget Firefox.
Free application must not be buggy
Comment 109•16 years ago
|
||
(In reply to comment #108)
> Firefox is currently the only application on MacOSX Intel 10.5.6 that have this
> bug.
This will be fixed in the next beta version of Firefox 3.1 and for Firefox 3.0.7. Please stay tuned.
Reporter | ||
Comment 110•16 years ago
|
||
I will check 10.6 the next time I am in the lab and report back.
You need to log in
before you can comment on or make changes to this bug.
Description
•