Closed Bug 457939 Opened 16 years ago Closed 15 years ago

messengerWindow needs an attribute to indicate the layout view type (classic, wide or vertical) for CSS selection proposes

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b2

People

(Reporter: ShareBird, Assigned: ShareBird)

Details

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.3pre) Gecko/2008091306 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080928120750 Shredder/3.0b1pre

On Thunderbird 2.0 it was possible to differentiate if the type of layout in use with some CSS "exercises" like:

/*classic view*/
#mailContentWrapper[orient="horizontal"] #messagesBox[orient="vertical"]{}
/*wide view*/
#mailContentWrapper[orient="vertical"] #messagesBox[orient="vertical"]{}
/*vertical view*/
#mailContentWrapper[orient="horizontal"] #messagesBox[orient="horizontal"]{}

Now, this approach doesn't work anymore. Actually it would be better if messengerWindow (or maybe the tabmail) would have an attribute indicating which view is in use to make possible to skin elements inside it (tabmail) according to the view (for example, margins, paddings and so on)

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Yeah, if the goal is to be able to style at the level of messagesBox (or anywhere above messagepanebox), I don't see any way with bug 389730, since now the only difference is which element is messagepanebox's parent.

I vote that someone else fixes it, though, since offhand I can't think of any elegant way.
Status: UNCONFIRMED → NEW
Component: General → Mail Window Front End
Ever confirmed: true
OS: Windows XP → All
QA Contact: general → front-end
Hardware: PC → All
Version: unspecified → Trunk
(In reply to comment #1)
> Yeah, if the goal is to be able to style at the level of messagesBox (or
> anywhere above messagepanebox), I don't see any way with bug 389730, since now
> the only difference is which element is messagepanebox's parent.
> 
> I vote that someone else fixes it, though, since offhand I can't think of any
> elegant way.


Thank you Phil for commenting and pointing to that bug. I'm not the expert for this stuff, but maybe something like:

Index: msgMail3PaneWindow.js
===================================================================
--- msgMail3PaneWindow.js	(revision 49)
+++ msgMail3PaneWindow.js	(working copy)
@@ -648,8 +648,11 @@
 // aMsgWindowInitialized: false if we are calling from the onload handler, otherwise true
 function UpdateMailPaneConfig(aMsgWindowInitialized) {
   const dynamicIds = ["messagesBox", "mailContent", "threadPaneBox"];
-  var desiredId = dynamicIds[gPrefBranch.getIntPref("mail.pane_config.dynamic")];
+	var layoutView = gPrefBranch.getIntPref("mail.pane_config.dynamic");
+  var desiredId = dynamicIds[layoutView];
   var messagePane = GetMessagePane();
+	var layout;
+	var mailContent = document.getElementById("mailContent");
   if (messagePane.parentNode.id != desiredId) {
     ClearAttachmentList();
     var messagePaneSplitter = GetThreadAndMessagePaneSplitter();
@@ -670,6 +673,20 @@
         gDBView.reloadMessage();
     }
   }
+	switch (layoutView) {
+		case 0:
+		layout = "standard";
+		break;
+		case 1:
+		layout ="wide";
+		break;
+		case 2:
+		layout = "vertical";
+		break;
+	}
+	if(mailContent) {
+		mailContent.setAttribute("layout", layout);
+	}
 }
 
 const MailPrefObserver = {


could solve the problem. (??)
Attached patch proposed patch (obsolete) — Splinter Review
This patch adds a "layout" attribute to mailContent indicating which layout is being used.
Attachment #354209 - Flags: review?(philringnalda)
Attached patch patch v1.1 (obsolete) — Splinter Review
added white-space after "if" as suggested by JST Reviewer Simulacrum...
Attachment #354209 - Attachment is obsolete: true
Attachment #354209 - Flags: review?(philringnalda)
Attachment #354213 - Flags: review?(philringnalda)
Assignee: nobody → pardal
Comment on attachment 354213 [details] [diff] [review]
patch v1.1

Rather than using a switch, you can just use another const array like dynamicIds to hold the layout names, and then since you want to set your layout attribute whether or not parentNode.id != desiredId, go ahead and set it up there at the top of the function, rather than separating your declarations from your use of them - nothing good for you can happen by letting other code get between when you declare a var and where you use it.

You can also skip the if check for mailContent: if we're switching layouts when mailContent isn't in the document, we want (and should certainly expect) error messages screaming that fact at the skies. Just assume, like desiredParent does, that you can forge ahead with document.getElementById("mailContent").setAttribute("layout", layouts[desiredId]) without checking.
Attachment #354213 - Flags: review?(philringnalda) → review-
Attached patch patch v1.2Splinter Review
Thanks. I've modified the patch.
Attachment #354213 - Attachment is obsolete: true
Attachment #355334 - Flags: review?(philringnalda)
Attachment #355334 - Flags: review?(philringnalda) → review+
Comment on attachment 355334 [details] [diff] [review]
patch v1.2

r=me, with a little wrapping I already did, since I accidentally fed you a line more than 80 characters long.
http://hg.mozilla.org/comm-central/rev/d931814567e5
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b2
You need to log in before you can comment on or make changes to this bug.