Closed Bug 103594 Opened 20 years ago Closed 20 years ago

[CASCADE][review]Clean up colors prefs code in nsPresShell

Categories

(Core :: CSS Parsing and Computation, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla0.9.8

People

(Reporter: pierre, Assigned: pierre)

References

Details

(Keywords: testcase)

Attachments

(3 files)

We should remove the PREFS_USE_OVERRIDE code in nsPresShell and follow what is 
described in the comments: 

//  - OVERRIDE is better for text and bg colors, but bad for link colors,
// so eventually, we should probably have an agent and an override and 
// put the link colors in the agent and the text and bg colors in the override,
Status: NEW → ASSIGNED
Depends on: 43220
Priority: -- → P3
Target Milestone: --- → mozilla0.9.6
In bug 103517, fantasai wrote: "Pref style rules should be inserted at the 
beginning of the User level; right now nsPresShell adds them as UA rules."
*** Bug 103517 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.6 → mozilla0.9.8
Marc/Daniel: please r/sr
Summary: Clean up colors prefs code in nsPresShell → [review]Clean up colors prefs code in nsPresShell
Attached patch patchSplinter Review
Attached file testcase
Pierre - does your patch handle this testcase correctly?
Keywords: testcase
Summary: [review]Clean up colors prefs code in nsPresShell → [CASCADE][review]Clean up colors prefs code in nsPresShell
Comment on attachment 61968 [details] [diff] [review]
patch

r=glazman
Attachment #61968 - Flags: review+
fantasai: the :root{} rule doesn't do anything even if it changed to !important.  
The testcase works fine though when the selector is changed to 'body'.
I suspected that would happen, since you're appending the stylesheet instead of 
prepending it. Try replacing
  +        mStyleSet->AppendUserStyleSheet(mPrefStyleSheet);
with
  +        mStyleSet->InsertUserStyleSheetBefore(mPrefStyleSheet,
  +                                        mStyleSet->GetUserStyleSheetAt(0));
I think the testcase should work then.

The reason it works with body is because the body is a child of :root--no matter 
how specific the rule is on :root, it will never override a color on the body.
(If you use body instead of :root and make the <head> and <title> display block, 
you'll see that the prefs color is set on the :root, because the <head> still 
has the prefs color.)
Attachment #61968 - Flags: superreview+
Comment on attachment 61968 [details] [diff] [review]
patch

sr=attinasi
fixed

checked in with mStyleSet->InsertUserStyleSheetBefore(mPrefStyleSheet, nsnull)
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Did this cause a 10ms jump in page load time?
on linux, i tested the testcase attachment 62200 [details] .....
...this is what i saw ...

TEST 1 : 
The text is blue on an aqua background. The entire body has the aqua background.

TEST 2:
The text is red on an aqua background, but, the aqua background is not painted
ont the entire body ....... only text background gets the aqua background ...
remaining page has a white background .

see what i am mean in the attacheded image.


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