Changing the print orientation requires a restart of TB

RESOLVED FIXED in Thunderbird 3

Status

Thunderbird
General
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Boying Lu, Assigned: Boying Lu)

Tracking

Trunk
Thunderbird 3

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 6 obsolete attachments)

(Assignee)

Description

10 years ago
====================
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...
(Assignee)

Comment 1

10 years ago
Created attachment 306235 [details] [diff] [review]
patch
Assignee: nobody → brian.lu
Status: NEW → ASSIGNED
Attachment #306235 - Flags: review?(bienvenu)
(Assignee)

Updated

10 years ago
Attachment #306235 - Attachment is obsolete: true
Attachment #306235 - Flags: review?(bienvenu)
(Assignee)

Comment 2

10 years ago
Created attachment 306447 [details] [diff] [review]
only affect TB
Attachment #306447 - Flags: review?(bienvenu)

Updated

10 years ago
OS: Linux → All
Hardware: PC → All
(Assignee)

Updated

10 years ago
Attachment #306447 - Flags: review?(bienvenu) → review?(neil)

Comment 3

10 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)
(Assignee)

Comment 4

10 years ago
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)
(Assignee)

Comment 5

10 years ago
Created attachment 309076 [details] [diff] [review]
[checked in] fixed in both e-mail and address book
(Assignee)

Updated

10 years ago
Attachment #309076 - Flags: review?(iann_bugzilla)

Comment 6

10 years ago
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+
(Assignee)

Updated

10 years ago
Attachment #309076 - Flags: superreview?(neil)

Comment 7

10 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+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
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
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3
(Assignee)

Comment 9

10 years ago
Created attachment 311520 [details] [diff] [review]
new patch

The new patch is based on Neil's comment
Attachment #311520 - Flags: review?(iann_bugzilla)

Comment 10

10 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

10 years ago
Thanks a lot.  I'll make a new patch 
(Assignee)

Updated

10 years ago
Attachment #311520 - Attachment is obsolete: true
(Assignee)

Comment 12

10 years ago
Created attachment 311949 [details] [diff] [review]
patch for both thunderbird and seamonkey
(Assignee)

Updated

10 years ago
Attachment #311949 - Flags: review?(iann_bugzilla)

Comment 13

10 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

10 years ago
Created attachment 312661 [details] [diff] [review]
patch with empty lines removed
Attachment #312661 - Flags: superreview?(neil)

Comment 15

10 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+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed

Comment 16

10 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?
(Assignee)

Comment 17

10 years ago
ok i'll take a look at this
Keywords: checkin-needed
(Assignee)

Updated

10 years ago
Attachment #312661 - Attachment is obsolete: true
(Assignee)

Comment 18

10 years ago
Created attachment 316193 [details] [diff] [review]
new patch based on Neil's comments
Attachment #316193 - Flags: review?(neil)

Comment 19

10 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...
(Assignee)

Updated

10 years ago
Attachment #316193 - Attachment is obsolete: true
Attachment #316193 - Flags: review?(neil)
(Assignee)

Comment 20

10 years ago
Created attachment 316343 [details] [diff] [review]
[checked in] patch for both thunderbird and seamonkey
Attachment #316343 - Flags: review?(neil)

Updated

10 years ago
Attachment #316343 - Flags: review?(neil) → review+
(Assignee)

Updated

10 years ago
Keywords: checkin-needed
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 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 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
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.