1470733 - Quote side bar incorrectly styled in compose window: Too thin and wrong colour

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement)

Description

[Jorg K (CEST = GMT+2)]

As per PM to assignee ;-) - TB 60 beta already affected.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: Bug1470733-quoteBorderWidth.patch

This fixes the border width for me. Additionally I cleaned the code a bit (removed the border-radius and changed a bit of the spacing between the quote level borders).

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 8987331 [details] [diff] [review]
Bug1470733-quoteBorderWidth.patch

Review of attachment 8987331 [details] [diff] [review]:
-----------------------------------------------------------------

This works for me, but I'd like to understand why so many changes are required.

::: mail/themes/windows/mail/messageQuotes.css
@@ +43,4 @@
>    border-color: rgb(114,159,207); /* Sky Blue 1 */
>  }
>  
> +blockquote[type=cite] > blockquote {

Why is it OK to leave off the [type=cite] on the second one? So why not:

blockquote[type=cite] > blockquote[type=cite]

And why change the selector to > ?

Oh, I see, Mac was always like this. But is that correct?

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: Bug1470733-quoteBorderWidth.patch - JK

Windows-only with only body prepended. This works, too.

Comment 4

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #2)
> Comment on attachment 8987331 [details] [diff] [review]
> Bug1470733-quoteBorderWidth.patch
> 
> Review of attachment 8987331 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This works for me, but I'd like to understand why so many changes are
> required.
> 
> ::: mail/themes/windows/mail/messageQuotes.css
> @@ +43,4 @@
> >    border-color: rgb(114,159,207); /* Sky Blue 1 */
> >  }
> >  
> > +blockquote[type=cite] > blockquote {
> 
> Why is it OK to leave off the [type=cite] on the second one? So why not:

I copied it from osx stylesheet and there it worked always. It seems, when we are in a blockquote[type=cite] all blockquote children are then of type="cite".

> blockquote[type=cite] > blockquote[type=cite]
> 
> And why change the selector to > ?

It's faster. Then it checks only the direct parent if it's a blockquote. Without '>' it could be every parent in the DOM tree and the parser checks every parent up to the root if it's a blockquote[type=cite].

> Oh, I see, Mac was always like this. But is that correct?

It worked until now, so I think yes.

Comment 5

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #3)
> Created attachment 8987336 [details] [diff] [review]
> Bug1470733-quoteBorderWidth.patch - JK
> 
> Windows-only with only body prepended. This works, too.

Only the first blockquote[type=cite] level needs the 'body' added. All others are more specific than the one from html.css.

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #4)
> I copied it from osx stylesheet and there it worked always. It seems, when
> we are in a blockquote[type=cite] all blockquote children are then of
> type="cite".
Well, sure, in anything we produce, but nothing stops people creating blockquotes which aren't cites. It would be more correct to check the way Windows did.

> > And why change the selector to > ?
> It's faster. Then it checks only the direct parent if it's a blockquote.
> Without '>' it could be every parent in the DOM tree and the parser checks
> every parent up to the root if it's a blockquote[type=cite].
Yes, but it's also safer. And in practice, there wouldn't be much to search. For the first level, the parent is the body. And for the level 2+ the parent is another quote. Only weird cases will cause more searching. Again, I prefer the Windows way.

> > Oh, I see, Mac was always like this. But is that correct?
> It worked until now, so I think yes.
I think I can construct something where the Mac code won't do the right thing.

(In reply to Richard Marti (:Paenglab) from comment #5)
> Only the first blockquote[type=cite] level needs the 'body' added. All
> others are more specific than the one from html.css.
I was just an example.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: AccountManager.patch

Okay, is this better. Changed osx to be the same as the other two platforms.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: Bug1470733-quoteBorderWidth.patch

Sorry, wrong patch.

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8987365 [details] [diff] [review]
Bug1470733-quoteBorderWidth.patch

Review of attachment 8987365 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks that works for me on Windows and should work on Linux equally. Mac is still mysteriously different.

::: mail/themes/osx/mail/messageQuotes.css
@@ +33,3 @@
>    color: blue !important;
>    border-color: blue !important;
> +  border-inline-start-width: 2px;

Why is the CSS for Mac so different and where does border-inline-start-width: 2px; suddenly come from when it wasn't there before?

@@ +39,5 @@
>    color: green !important;
>    border-color: green !important;
>  }
>  
> +blockquote[type=cite] blockquote[type=cite] blockquote[type=cite] {

Why does Mac only have three levels when the other platforms have five? Should we increase that?

Comment 10

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #9)
> Comment on attachment 8987365 [details] [diff] [review]
> Bug1470733-quoteBorderWidth.patch
> 
> Review of attachment 8987365 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why is the CSS for Mac so different and where does
> border-inline-start-width: 2px; suddenly come from when it wasn't there
> before?

It was always 1px before. Now it's 2px like in message reading. The difference is, that Mac has the border only on the left side.
 
> Why does Mac only have three levels when the other platforms have five?
> Should we increase that?

This is since long time like this. I decided to leave it like it is now. If we want to extend it, we'd need to choose the border colors as Mac uses different to the other platforms.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5d4f4fe4d4e6
Fix the quote border width in the compose window. r=jorgk

Comment 12

[Jorg K (CEST = GMT+2)]

Comment on attachment 8987365 [details] [diff] [review]
Bug1470733-quoteBorderWidth.patch

I hope that fixes the TB 60 beta problem, too.

Comment 13

[Jorg K (CEST = GMT+2)]

TB 60 beta 9 (BETA_60_CONTINUATION branch):
https://hg.mozilla.org/releases/comm-beta/rev/516fa9132deb4e7a25193203075a89c6188ee05b

Comment 14

[Jorg K (CEST = GMT+2)]

Works fine in the beta.

Comment 15

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/releases/comm-esr60/rev/0785e6931ff0