Last Comment Bug 663375 - css media="print" text-decoration styles not being processed correctly
: css media="print" text-decoration styles not being processed correctly
Status: RESOLVED FIXED
: testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: Mats Palmgren (:mats)
:
Mentors:
http://www.armonkplayers.org/reservat...
Depends on: 403524
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-10 06:53 PDT by Ronald Aaronson
Modified: 2011-08-10 08:26 PDT (History)
6 users (show)
mats: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Reporter's testcase (standards mode) (476 bytes, text/html)
2011-06-12 20:26 PDT, Mats Palmgren (:mats)
no flags Details
Reporter's testcase in Quirks mode (460 bytes, text/html)
2011-06-12 20:27 PDT, Mats Palmgren (:mats)
no flags Details
wip1 (11.79 KB, patch)
2011-06-12 20:30 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review
wip2 (10.93 KB, patch)
2011-06-12 20:46 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review
fix (13.64 KB, patch)
2011-08-08 13:44 PDT, Mats Palmgren (:mats)
dbaron: review+
Details | Diff | Review
fix with nits fixed (13.58 KB, patch)
2011-08-09 08:20 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Review

Description Ronald Aaronson 2011-06-10 06:53:15 PDT
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 2011-06-10 08:48:39 PDT
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 XtC4UaLL [:xtc4uall] 2011-06-10 08:57:07 PDT
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 2011-06-10 09:23:19 PDT
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 (:mats) 2011-06-12 20:23:11 PDT
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 (:mats) 2011-06-12 20:26:45 PDT
Created attachment 538800 [details]
Reporter's testcase (standards mode)
Comment 6 Mats Palmgren (:mats) 2011-06-12 20:27:43 PDT
Created attachment 538801 [details]
Reporter's testcase in Quirks mode
Comment 7 Mats Palmgren (:mats) 2011-06-12 20:30:43 PDT
Created attachment 538802 [details] [diff] [review]
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-7 (review requests must explain patch) 2011-06-12 20:43:36 PDT
Likely best to hold off on this until bug 403524 lands.
Comment 9 Mats Palmgren (:mats) 2011-06-12 20:46:02 PDT
Created attachment 538804 [details] [diff] [review]
wip2
Comment 10 Mats Palmgren (:mats) 2011-08-08 13:44:50 PDT
Created attachment 551559 [details] [diff] [review]
fix

updated to tip
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-08-08 14:57:55 PDT
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-7 (review requests must explain patch) 2011-08-08 14:59:34 PDT
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 (:mats) 2011-08-09 08:20:44 PDT
Created attachment 551770 [details] [diff] [review]
fix with nits fixed
Comment 15 :Ehsan Akhgari (busy, don't ask for review please) 2011-08-10 08:26:09 PDT
http://hg.mozilla.org/mozilla-central/rev/e9f6607a3990

Note You need to log in before you can comment on or make changes to this bug.