Closed
Bug 420050
Opened 16 years ago
Closed 16 years ago
Changing the print orientation requires a restart of TB
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3
People
(Reporter: eagle.lu, Assigned: eagle.lu)
Details
Attachments
(2 files, 6 obsolete files)
1.89 KB,
patch
|
iannbugzilla
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
8.11 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
==================== Steps to re-create:- ==================== 1. Open thunderbird 2. Goto File -> Page Setup 3. Change the orientaiton format (e.g. from potrait to landscape in this case) & click OK 4. Select an email and print it 5. You will see it prints in the original orientaion. (e.g. portrait in this case) 6. Restart thunderbird and print the email again and you will see it prints in the newly selected orientation (e.g. landscape in this case) 7. Changing back to the original orientation (e.g. portrait in this case) requires another restart of the application and so on...
Attachment #306235 -
Attachment is obsolete: true
Attachment #306235 -
Flags: review?(bienvenu)
Attachment #306447 -
Flags: review?(bienvenu)
Updated•16 years ago
|
OS: Linux → All
Hardware: PC → All
Attachment #306447 -
Flags: review?(bienvenu) → review?(neil)
Comment 3•16 years ago
|
||
Comment on attachment 306447 [details] [diff] [review] only affect TB Ian, you touched printing last ;-) Does this problem affect addressbook too?
Attachment #306447 -
Flags: review?(neil) → review?(iann_bugzilla)
Comment on attachment 306447 [details] [diff] [review] only affect TB The bug also affect address book. But the patch doesn't work for address book. I'll make a new patch
Attachment #306447 -
Attachment is obsolete: true
Attachment #306447 -
Flags: review?(iann_bugzilla)
Attachment #309076 -
Flags: review?(iann_bugzilla)
Comment on attachment 309076 [details] [diff] [review] [checked in] fixed in both e-mail and address book This is fine, SeaMonkey will also need to be patched though. It would be good to see if we can get this and the SM patch onto the branch too.
Attachment #309076 -
Flags: review?(iann_bugzilla) → review+
Attachment #309076 -
Flags: superreview?(neil)
Comment 7•16 years ago
|
||
Comment on attachment 309076 [details] [diff] [review] [checked in] fixed in both e-mail and address book Two questions, which you may want to fix now or leave for a followup bug: 1. Do we need to keep the print settings in a global any more? 2. If not, do we actually need to access the print settings here or should we do it in msgPrintEngine.js?
Attachment #309076 -
Flags: superreview?(neil) → superreview+
Keywords: checkin-needed
Comment 8•16 years ago
|
||
Checking in mail/components/addrbook/content/addressbook.js; /cvsroot/mozilla/mail/components/addrbook/content/addressbook.js,v <-- addressbook.js new revision: 1.46; previous revision: 1.45 done Checking in mail/base/content/mailWindowOverlay.js; /cvsroot/mozilla/mail/base/content/mailWindowOverlay.js,v <-- mailWindowOverlay.js new revision: 1.190; previous revision: 1.189 done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3
The new patch is based on Neil's comment
Attachment #311520 -
Flags: review?(iann_bugzilla)
Comment 10•16 years ago
|
||
Comment on attachment 311520 [details] [diff] [review] new patch >Index: mail/components/addrbook/content/addressbook.js >=================================================================== >@@ -365,13 +364,12 @@ > } > } > >- gPrintSettings = PrintUtils.getPrintSettings(); > Nit: Remove the extra blank line, only need one. >@@ -410,12 +408,11 @@ > var abURIArr = uri.split("://"); > var printUrl = "addbook://" + abURIArr[0] + "/" + abURIArr[1] + "?action=print" > >- gPrintSettings = PrintUtils.getPrintSettings(); > Nit: Remove the extra blank line, only need one. >Index: mailnews/base/resources/content/msgPrintEngine.js >=================================================================== >- if (window.arguments.length > 5) { >- printEngine.setMsgType(window.arguments[5]); >+ if (window.arguments.length > 4) { >+ printEngine.setMsgType(window.arguments[4]); >+ // printEngine.setMsgType(window.arguments[5]); Nit: Remove the commented out line. r- as with this change to the arguments sent to msgPrintEngine, you will completely break printing from mailnews and addressbook in SeaMonkey. Lines to patch in /mailnews/addrbook/resources/content/addressbook.js and /mailnews/base/resources/content/mailWindowOverlay.js should be identical to TB equivalents - if you need help/testing on SM let me know.
Attachment #311520 -
Flags: review?(iann_bugzilla) → review-
Assignee | ||
Comment 11•16 years ago
|
||
Thanks a lot. I'll make a new patch
Attachment #311520 -
Attachment is obsolete: true
Assignee | ||
Comment 12•16 years ago
|
||
Attachment #311949 -
Flags: review?(iann_bugzilla)
Comment 13•16 years ago
|
||
Comment on attachment 311949 [details] [diff] [review] patch for both thunderbird and seamonkey >Index: mail/components/addrbook/content/addressbook.js >=================================================================== >@@ -365,13 +364,12 @@ > } > } > >- gPrintSettings = PrintUtils.getPrintSettings(); > You still need to remove the extra blank line here, you only need the one. > printEngineWindow = window.openDialog("chrome://messenger/content/msgPrintEngine.xul", > "", > "chrome,dialog=no,all", > totalCard, selectionArray, statusFeedback, >- gPrintSettings, doPrintPreview, msgType); >+ doPrintPreview, msgType); > > return; > } >@@ -410,12 +408,11 @@ > var abURIArr = uri.split("://"); > var printUrl = "addbook://" + abURIArr[0] + "/" + abURIArr[1] + "?action=print" > >- gPrintSettings = PrintUtils.getPrintSettings(); > You still need to remove the extra blank line here, you only need the one. > printEngineWindow = window.openDialog("chrome://messenger/content/msgPrintEngine.xul", > "", > "chrome,dialog=no,all", >- 1, [printUrl], statusFeedback, gPrintSettings, doPrintPreview, msgType); >+ 1, [printUrl], statusFeedback, doPrintPreview, msgType); > > return; > } r=me with those changes.
Attachment #311949 -
Flags: review?(iann_bugzilla) → review+
Assignee | ||
Comment 14•16 years ago
|
||
Attachment #312661 -
Flags: superreview?(neil)
Comment 15•16 years ago
|
||
Comment on attachment 312661 [details] [diff] [review] patch with empty lines removed >+ printSettings = PrintUtils.getPrintSettings(); >+ if (printSettings) { >+ printSettings.isCancelled = false; >+ } Nit: This is possibly not the ideal place to put this code block.
Attachment #312661 -
Flags: superreview?(neil) → superreview+
Keywords: checkin-needed
Comment 16•16 years ago
|
||
(In reply to comment #15) > (From update of attachment 312661 [details] [diff] [review]) > >+ printSettings = PrintUtils.getPrintSettings(); > >+ if (printSettings) { > >+ printSettings.isCancelled = false; > >+ } > Nit: This is possibly not the ideal place to put this code block. > Boying Lu are you going to look at this before checkin?
Attachment #312661 -
Attachment is obsolete: true
Assignee | ||
Comment 18•16 years ago
|
||
Attachment #316193 -
Flags: review?(neil)
Comment 19•16 years ago
|
||
(In reply to comment #18) > Created an attachment (id=316193) [details] > new patch based on Neil's comments > Don't forget the SeaMonkey part of the patch...
Attachment #316193 -
Attachment is obsolete: true
Attachment #316193 -
Flags: review?(neil)
Assignee | ||
Comment 20•16 years ago
|
||
Attachment #316343 -
Flags: review?(neil)
Updated•16 years ago
|
Attachment #316343 -
Flags: review?(neil) → review+
Keywords: checkin-needed
Comment 21•16 years ago
|
||
Comment on attachment 309076 [details] [diff] [review] [checked in] fixed in both e-mail and address book Note, this patch was already checked in by reed on 2008-03-18 12:39.
Attachment #309076 -
Attachment description: fixed in both e-mail and address book → [checked in] fixed in both e-mail and address book
Comment 22•16 years ago
|
||
Comment on attachment 311949 [details] [diff] [review] patch for both thunderbird and seamonkey I'm fairly sure this one is obsolete.
Attachment #311949 -
Attachment is obsolete: true
Comment 23•16 years ago
|
||
Comment on attachment 316343 [details] [diff] [review] [checked in] patch for both thunderbird and seamonkey Checking in mail/base/content/mailWindowOverlay.js; /cvsroot/mozilla/mail/base/content/mailWindowOverlay.js,v <-- mailWindowOverlay.js new revision: 1.196; previous revision: 1.195 done Checking in mail/components/addrbook/content/addressbook.js; /cvsroot/mozilla/mail/components/addrbook/content/addressbook.js,v <-- addressbook.js new revision: 1.47; previous revision: 1.46 done Checking in mailnews/base/resources/content/msgPrintEngine.js; /cvsroot/mozilla/mailnews/base/resources/content/msgPrintEngine.js,v <-- msgPrintEngine.js new revision: 1.24; previous revision: 1.23 done Checking in mailnews/base/resources/content/mailWindowOverlay.js; /cvsroot/mozilla/mailnews/base/resources/content/mailWindowOverlay.js,v <-- mailWindowOverlay.js new revision: 1.267; previous revision: 1.266 done Checking in mailnews/addrbook/resources/content/addressbook.js; /cvsroot/mozilla/mailnews/addrbook/resources/content/addressbook.js,v <-- addressbook.js new revision: 1.137; previous revision: 1.136
Attachment #316343 -
Attachment description: patch for both thunderbird and seamonkey → [checked in] patch for both thunderbird and seamonkey
Updated•16 years ago
|
Keywords: checkin-needed
You need to log in
before you can comment on or make changes to this bug.
Description
•