Last Comment Bug 546941 - Scrollbars are not being update using dss.enabled
: Scrollbars are not being update using dss.enabled
Status: RESOLVED FIXED
: fixed1.9.0.19, regression
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: 1.9.2 Branch
: All All
: -- normal (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (TPAC)
:
Mentors:
Depends on:
Blocks: CVE-2010-0169
  Show dependency treegraph
 
Reported: 2010-02-18 07:20 PST by Pardal Freudenthal (:ShareBird)
Modified: 2010-03-03 16:27 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.2-fixed
.9-fixed


Attachments
Proposed fix (4.97 KB, patch)
2010-02-18 10:29 PST, Boris Zbarsky [:bz] (TPAC)
dbaron: review+
Details | Diff | Splinter Review
With that change (6.18 KB, patch)
2010-02-18 13:02 PST, Boris Zbarsky [:bz] (TPAC)
mbeltzner: approval1.9.2.2+
mbeltzner: approval1.9.1.9+
mbeltzner: approval1.9.0.19+
Details | Diff | Splinter Review

Description Pardal Freudenthal (:ShareBird) 2010-02-18 07:20:04 PST
I've written a small extension to make possible to switch themes without a restart. 
It basically uses the dss.enabled feature and the reloadChrome method from nsIChromeRegistry: http://mxr.mozilla.org/mozilla-central/source/content/base/public/nsIChromeRegistry.idl#87

The extension is available at: https://addons.mozilla.org/en-US/firefox/addon/61769

With Namoroka builds after 2010-02-13 the scrollbars are not being updated as they should and have done until build 2010-02-12.
This issues is only present on Namoroka, not on Minefield.

The pushlog range of this regression is: http://hg.mozilla.org/releases/mozilla-1.9.2/pushloghtml?fromchange=7723073d61b2&tochange=ebba043bb505

So it seems it is a regression from Bug 535806
Comment 1 Boris Zbarsky [:bz] (TPAC) 2010-02-18 08:51:59 PST
Ah, I bet the chrome registry uses a case-insensitive CSSLoader (which means that it was pretty broken to start with in terms of its stylesheet reloading).
Comment 2 Boris Zbarsky [:bz] (TPAC) 2010-02-18 10:29:56 PST
Created attachment 427611 [details] [diff] [review]
Proposed fix
Comment 3 Boris Zbarsky [:bz] (TPAC) 2010-02-18 10:31:14 PST
Should probably land this on older branches too, right?
Comment 4 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2010-02-18 12:14:01 PST
Comment on attachment 427611 [details] [diff] [review]
Proposed fix

>   static void LoadSheet(nsIURI* aURI, nsCOMPtr<nsICSSStyleSheet> &aSheet,
>-                        PRBool aEnableUnsafeRules);
>+                        PRBool aEnableUnsafeRules,
>+                        PRBool aUseCaseSensitiveLoader = PR_FALSE);
> 

Can you avoid the default value and make all callers pass the boolean explicitly?

r=dbaron
Comment 5 Boris Zbarsky [:bz] (TPAC) 2010-02-18 13:02:40 PST
Created attachment 427637 [details] [diff] [review]
With that change

Requesting branch approvals.  This patch is not necessary (or even possible) on trunk.
Comment 6 Mike Beltzner [:beltzner, not reading bugmail] 2010-02-22 10:21:46 PST
Comment on attachment 427637 [details] [diff] [review]
With that change

a=beltzner for all branches, thanks, bz!
Comment 7 Boris Zbarsky [:bz] (TPAC) 2010-02-23 21:24:30 PST
Pushed:
  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e87a0bfee150
  http://hg.mozilla.org/releases/mozilla-1.9.1/rev/ed881b968aef

Checked into CVS:
Checking in chrome/src/nsChromeRegistry.cpp;
/cvsroot/mozilla/chrome/src/nsChromeRegistry.cpp,v  <--  nsChromeRegistry.cpp
new revision: 1.370; previous revision: 1.369
done
Checking in layout/style/nsLayoutStylesheetCache.h;
/cvsroot/mozilla/layout/style/nsLayoutStylesheetCache.h,v  <--  nsLayoutStylesheetCache.h
new revision: 1.9; previous revision: 1.8
done
Checking in layout/style/nsLayoutStylesheetCache.cpp;
/cvsroot/mozilla/layout/style/nsLayoutStylesheetCache.cpp,v  <--  nsLayoutStylesheetCache.cpp
new revision: 1.15; previous revision: 1.14
Comment 8 Jordan Gutterman 2010-02-23 23:30:05 PST
>   http://hg.mozilla.org/releases/mozilla-1.9.2/rev/e87a0bfee150

This looks like it was made on a relbranch and not the default branch, right?
Comment 9 Boris Zbarsky [:bz] (TPAC) 2010-02-23 23:36:52 PST
Er... yes.  I have no idea how that happened, but it's wrong.  Fixing now.
Comment 10 Boris Zbarsky [:bz] (TPAC) 2010-02-23 23:39:38 PST
Backed out from the relbranch:
  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4bd1755a1619

Pushed to default branch:
  http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7a9b9969e527
Comment 11 Pardal Freudenthal (:ShareBird) 2010-03-03 06:30:49 PST
Thank you Boris and Mike!
Is it possible to land this patch also on comm-1.9.1 branch?
Comment 12 Boris Zbarsky [:bz] (TPAC) 2010-03-03 09:12:22 PST
I checked this in on mozilla-1.9.1.  I would think that comm-1.9.1 just pulls that.  Does it not?
Comment 13 Pardal Freudenthal (:ShareBird) 2010-03-03 14:58:04 PST
Hmmm... I've checked again and it seems that it was landed:
http://mxr.mozilla.org/mozilla1.9.2/source/chrome/src/nsChromeRegistry.cpp#841
http://mxr.mozilla.org/comm-1.9.1/source/mozilla/chrome/src/nsChromeRegistry.cpp#899

But I'm still having this issue on Thunderbird Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8) Gecko/20100302 Shredder/3.0.4pre
Comment 14 Boris Zbarsky [:bz] (TPAC) 2010-03-03 15:20:25 PST
This landed after Gecko 1.9.1.8 branched.  That's why the status1.9.1 flag above is set to ".9-fixed" (as in, fixed for 1.9.1.9).
Comment 15 Pardal Freudenthal (:ShareBird) 2010-03-03 16:27:39 PST
Ah... I understand. Thanks!

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