663375 - css media="print" text-decoration styles not being processed correctly

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Ronald Aaronson]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1

Not all properties of a style within a <style media="print"> tag are being processed, such as color, background-color and text-decoration.

Reproducible: Always

Steps to Reproduce:
1. Invoke http://www.armonkplayers.org/reservations/test.html
2. Observe page as displayed in browser.
3. Do a "Print Preview" or Print


Actual Results:  
The contents of the second table cell (BBB) is properly printed twice as large as the other cells, but the color, background-color and text-decoration properties are wrong.

Expected Results:  
The second cell should be white text on a black background and underlined.

The style sheets in question:

<style type="text/css">
.b {
  color: red;
  background-color: white;
  text-decoration: line-through;
}
</style>

<style type="text/css" media="print">
.b {
  color: white;
  background-color: black;
  text-decoration: underline;
  font-size: 200%
}
</style>

Comment 1

[Alice0775 White]

Works for me on
http://hg.mozilla.org/mozilla-central/rev/1619d8fc6416
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:7.0a1) Gecko/20110610 Firefox/7.0a1 ID:20110610030736

Check "Print Background (colors & images)" in File – Page Setup… dialog.

Comment 2

[(mostly gone) XtC4UaLL [:xtc4uall]]

WFM too against Mozilla/5.0 (Windows NT 5.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 ID:20110413222027 with "Print Background (colors & images)" checked.

Comment 3

[Ronald Aaronson]

I am sorry to have "spun your wheels".  I suppose the folks at Mozilla figure that if you are not honoring 'background-color', then you really cannot honor 'color' either.  But what is the rationale for ignoring 'text-decoration'?

So all in all, this is a print option that goes a bit beyond what I would expect.  Anyway, thanks for your time.

Comment 4

[Mats Palmgren (inactive)]

Seems like a valid bug to me.  We should do the same color mangling
for the text-decoration lines as we do for the text.

Comment 5

[Mats Palmgren (inactive)]

Attachment: Reporter's testcase (standards mode)

Comment 6

[Mats Palmgren (inactive)]

Attachment: Reporter's testcase in Quirks mode

Comment 7

[Mats Palmgren (inactive)]

Attachment: wip1

Something like this... should merge the nsLayoutUtils::GetColor with
part 2 in bug 312156 which does mostly the same thing.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Likely best to hold off on this until bug 403524 lands.

Comment 9

[Mats Palmgren (inactive)]

Attachment: wip2

Comment 10

[Mats Palmgren (inactive)]

Attachment: fix

updated to tip

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 551559 [details] [diff] [review]
fix

>-    } while (context && hasDecorationLines && (0 != decorMask));
>+    } while (0 != decorMask &&
>+             NS_SUCCEEDED(f->GetParentStyleContextFrame(presContext, &f, &isChild)) &&
>+             f);

GetParentStyleContextFrame is intended for use only when reconstructing the style contexts.  You either want to continue walking up the style context tree as this code was, or you should use nsLayoutUtils::GetParentOrPlaceholderFor.  (Though, really, this code should be entirely rewritten... or we should just find a way to get rid of nsTextBoxFrame.  Not for this patch, though.)

r=dbaron with that

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 551559 [details] [diff] [review]
fix

>-      const nscolor color = useOverride ? overrideColor
>-        : context->GetVisitedDependentColor(eCSSProperty_text_decoration_color);
>+      const nscolor color = useOverride ? overrideColor :
>+        nsLayoutUtils::GetColor(f, eCSSProperty_text_decoration_color);

And please leave the : on the following line.

Comment 13

[Mats Palmgren (inactive)]

Attachment: fix with nits fixed

Comment 14

[Mats Palmgren (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/e9f6607a3990

Comment 15

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/e9f6607a3990