Closed
Bug 463577
Opened 16 years ago
Closed 15 years ago
inline forwarded messages show too much html composition chrome
Categories
(Thunderbird :: Message Compose Window, defect, P2)
Thunderbird
Message Compose Window
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b2
People
(Reporter: davida, Assigned: philor)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 5 obsolete files)
4.77 KB,
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
in particular, the red dotted lines around the headers of forwarded messages stick out. They are not going to be edited in 99.97% of cases, and the fact that they're implemented as an HTML table should be almost hidden. I'll attach a WIP which works on mac only so far. Port to Qute is trivial when bryan and i settle on the css tweaks. (note: i don't think this _blocks_ b1, but i'm still targeting it for b1, and I think it does block tb3)
Flags: blocking-thunderbird3+
> They are not going to be edited in 99.97% of cases, and the fact
> that they're implemented as an HTML table should be almost hidden.
I wouldn't necessarily say that. We see quite a few posts in the forums where users want to clean up address lists when forwarding messages, and require some instructions how to edit or remove the "To:" header row in the table. I'm not sure how they would accomplish this without seeing the table borders to highlight the respective row(s) they want to remove.
The other question being, does the header information need to be in a table in the first place or are there other/better options?
Reporter | ||
Comment 2•16 years ago
|
||
Agreed, they should be editable for those .03% uses =) FYI, my patch shows the border on hover, which I think makes the task quite easily doable, without making the block so scary. I'd be fine w/ not using a table, btw -- but that sounds like a different patch. This is just low hanging cosmetic fruit.
> FYI, my patch shows the border on hover, which I think makes the task quite > easily doable, without making the block so scary. Ok, good. I didn't test your patch, but if it shows up when hovering over it this should be sufficient for the .03%+ users to find them. > I'd be fine w/ not using a table, btw -- but that sounds like a different > patch. This is just low hanging cosmetic fruit. I can't quite come up with any alternate idea, other than just inserting them as regular text, but maybe Bryan has some suggestions that would be feasible for a spin-off bug.
Updated•16 years ago
|
Attachment #346854 -
Attachment is patch: true
Attachment #346854 -
Attachment mime type: application/octet-stream → text/plain
Comment 6•16 years ago
|
||
I don't think this blocks b1 - if it should, please correct...
Target Milestone: Thunderbird 3.0b1 → Thunderbird 3.0b2
Reporter | ||
Comment 7•16 years ago
|
||
Assignee: nobody → david.ascher
Attachment #346854 -
Attachment is obsolete: true
Attachment #351001 -
Flags: ui-review?(clarkbw)
Attachment #351001 -
Flags: review?(bugzilla)
Comment 8•16 years ago
|
||
Comment on attachment 351001 [details] [diff] [review] v1 huge improvement to the current table border ugliness. I think we'll need to do a bit of research into what other clients are doing and expecting before making improvements to the actual layout.
Attachment #351001 -
Flags: ui-review?(clarkbw) → ui-review+
Comment 9•16 years ago
|
||
Comment on attachment 351001 [details] [diff] [review] v1 diff --git a/mail/themes/qute/mail/jar.mn b/mail/themes/qute/mail/jar.mn --- a/mail/themes/qute/mail/jar.mn +++ b/mail/themes/qute/mail/jar.mn @@ -48,6 +48,7 @@ skin/classic/messenger/addressbook/icons/secure-remote-addrbook.png (addrbook/secure-remote-addrbook.png) * skin/classic/messenger/messengercompose/messengercompose.css (compose/messengercompose.css) * skin/classic/messenger/messengercompose/editorOverlay.css (compose/editorOverlay.css) + skin/classic/messenger/messengercompose/body.css (compose/body.css) You're either missing body.css from the patch, or you're including it in qute where you don't need to.
Attachment #351001 -
Flags: review?(bugzilla) → review-
Reporter | ||
Updated•15 years ago
|
Severity: normal → minor
Priority: -- → P2
Reporter | ||
Comment 10•15 years ago
|
||
Updated patch. carrying forward ui-r.
Attachment #351001 -
Attachment is obsolete: true
Attachment #358963 -
Flags: ui-review+
Attachment #358963 -
Flags: review?(bugzilla)
Reporter | ||
Updated•15 years ago
|
Whiteboard: [needs review standard8]
Reporter | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•15 years ago
|
Attachment #358963 -
Attachment is patch: true
Attachment #358963 -
Attachment mime type: application/octet-stream → text/plain
Assignee | ||
Comment 11•15 years ago
|
||
Comment on attachment 358963 [details] [diff] [review] updated patch Unfortunately, to replace EditorContent.css, you must become EditorContent.css. This breaks displaying smileys as images, and being able to spot where you put a named anchor, and being able to tell that there's a form in something you forward or reply to, and seeing labels, and assuming -moz-force-broken-image-icon has a point, probably seeing your broken images. Much as you'll hate to do it, you need to either just copy-paste all of EditorContent.css and make your changes there, or overlay your stylesheet with more-specific or more-!important rules on the compose window with a % style jar.mn line. And cute as the .addOverrideStyleSheet thing is, it's way simpler to use an % override in the jar.mn to replace one chrome URL with another, and makes it so that third-party themes don't *have* to do their own version of body.css, while showing them how they can.
Attachment #358963 -
Flags: review?(bugzilla) → review-
Assignee | ||
Comment 12•15 years ago
|
||
(In reply to comment #11) > or overlay your stylesheet with > more-specific or more-!important rules on the compose window with a % style > jar.mn line. By which I clearly mean "I'm high as a kite, don't listen to me when I'm claiming you can somehow find a URL for the <editor> to % style."
Assignee | ||
Comment 13•15 years ago
|
||
While this doesn't break everything else in EditorContent.css (since it leaves it in), and doesn't mess with the joyous red for people happily writing email filled with tables by only touching the header table, it still kinda sucks, since the table:hover and tr:hover rules are pointless: you can't actually hover either one since you always have a th or td between you and the table and its rows, so you'll never see more than one cell with a border.
Assignee | ||
Comment 14•15 years ago
|
||
Oh, perhaps if I could manage to remember things, I'd remember that .bare-class:hover only applies to links (at least in quirks mode).
Attachment #359885 -
Attachment is obsolete: true
Attachment #359886 -
Flags: review?(mkmelin+mozilla)
Comment 15•15 years ago
|
||
Comment on attachment 359886 [details] [diff] [review] Slightly more >+ * The Original Code is. Poetic :) >+ * >+ * The Initial Developer of the Original Code is >+ * David Ascher <david.ascher@gmail.com>. >+ * Portions created by the Initial Developer are Copyright (C) 2___ >+ * the Initial Developer. All Rights Reserved. Needs some fixing too. How about showing all the table cell borders when you're hovering over the table? I'm not convinced about the one cell at a time, though overall it's better than status quo.
Assignee | ||
Comment 16•15 years ago
|
||
Comment on attachment 359886 [details] [diff] [review] Slightly more Oh yeah, that's not his license email, is it? I thought last night that we were stuck not being able to do every border, but I guess that's actually table.moz-email-headers-table:hover > tbody > tr > th/td, isn't it?
Attachment #359886 -
Attachment is obsolete: true
Attachment #359886 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Updated•15 years ago
|
Assignee: david.ascher → philringnalda
Whiteboard: [needs review standard8]
Assignee | ||
Comment 17•15 years ago
|
||
You can see why I didn't like the idea of applying the hover effect to all tables by just clicking in the fwd table to get the resizers and remove buttons, then grabbing one of the little tiny resizers, with the borders flickering on and off as you maneuver to something that requires just barely not-hovering the table. I'm okay with making the lives of people who compose table-filled mail _slightly_ miserable, but not that miserable.
Attachment #358963 -
Attachment is obsolete: true
Attachment #359961 -
Flags: review?(mkmelin+mozilla)
Comment 18•15 years ago
|
||
Comment on attachment 359961 [details] [diff] [review] Perhaps just the right amount Looks good to me! r=mkmelin, but please make the "The Original Code is." sentence say what it is :)
Attachment #359961 -
Flags: review?(mkmelin+mozilla) → review+
Assignee | ||
Comment 19•15 years ago
|
||
http://hg.mozilla.org/comm-central/rev/a358c7f1104b
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•