With the current MozMill trunk version you should run this test. The whole UI is broken and the editor is gone. Just execute the script with MozMill. Also see the screenshot.

For some reason reseting the prefs system removes inline CSS from the IDE, I fixed this by moving the inline css to the mozmill.css file. Committed revision 378.

I did a test with r378 but the problem is still there. The fonts don't screw up anymore but the editor window is still blowing away.

This is because the prefs reset is basically reloading the window, I don't know there is anything that I can do about this.

Boris or David, does one of you both have an idea why completely flushing and re-initializing the preferences system can result in such a broken window as you can see in attachment 364009 [details]. Is it a core issue we have here?

Broken in what sense? I have no idea what this function does, what preference-changed notifications it sends, what your expected and actual behavior is, or anything else that would maybe be needed to answer your question.

According to: http://www.xulplanet.com/references/xpcomref/ifaces/nsIPrefService.html void resetPrefs ( ) Called to completely flush and re-initialize the preferences system. I just ran this test multiple times in FF3 release and it works fine, however when I run it in minefield, the tests pass but the editor UI gets totally broken. This looks like its because of the same problem I fixed for the rest of the UI which is that that dynamically added css style properties get blown away. I have absolutely no idea ho to fix that, and I'm wondering if this is a regression in minefield?

Yes, I read that documentation. It's completely useless in terms of answering the "what does it _do_" question on the level on which Henrik's question needs it to be answered. If something changed in terms of behavior from Fx3 release to now, that certainly sounds like a regression. Can you give me steps to reproduce the problem or find a one-day regression range? > dynamically added css style properties get blown away Dynamically added how? Are they actually getting "blown away", or is the rendering just as if they were? In any case, the one-day range when the behavior changed would be a great start.

step 1: Open an HTML page with window.open in chrome mode step 2: Create an iframe within that document step 2: Use javascript to append a node to the iframe's dom step 3: Using javascript set the nodes classname to a class with some visual properties step 4: in the window do the following: service = Components.classes["@mozilla.org/preferences-service;1"]. getService(Components.interfaces.nsIPrefService); service.resetPrefs() In FF3 the attributes in that class stay rendered properly (or are re-applied to the node), but in minefield they do not. Accessing the style attributes directly are all set to default in both. Someone with more experience tracking down changes in the code base should take a stab at finding a day range, any volunteers?

Those steps to reproduce are extremely vague. It's much more helpful to say "download the attached test page" rather than "write a script that does X" because you can easily miss the key points about what *exactly* the script needs to do. (I've found it's not generally worth my time bothering to try to reproduce steps to reproduce that are that vague, because they usually don't work.)

Then I suppose the best repo case I can produce at the moment is: Install Mozmill trunk View the working editor Goto the shell tab Type: var service = Components.classes["@mozilla.org/preferences-service;1"]. getService(Components.interfaces.nsIPrefService); service.resetPrefs() Press enter Go back to the editor tab, observe the broken styling.

It may also be helpful to know that every piece of css was broken in the whole UI until I moved the styling out of the main html page an into its own .css file, now it's just inside the iframe that is generated by the editor.

So I took the following steps: 1. installed MozMill from https://addons.mozilla.org/en-US/firefox/addon/9018 (or is that not the trunk? If not, please provide a URL.) 2. restarted Firefox 3. went to Tools -> MozMill The "Editor" tab now looks selected, but it's blank. How do I tell that it is "working"? Or were my previous steps wrong? I don't see a "shell" tab.

My apologies, here is a mozmill trunk build from yesterday: http://mozmill.googlecode.com/files/mozmill-trunk-r372.xpi The directions should make a lot more sense now.

Can the steps for comment 9 be converted into a string that I can just evaluate in the Firefox JS console to reproduce the bug? Or is MozMill a necessary ingredient here?

I haven't tested it with FF3.0. I will run a test and check if it's a regression. Boris and David, sorry for wasting your time in-front. I'll report back when I've found the exact regression range. (In reply to comment #15) > Can the steps for comment 9 be converted into a string that I can just evaluate > in the Firefox JS console to reproduce the bug? Or is MozMill a necessary > ingredient here? You only have to install MozMill and open up its IDE. The above string can be entered in the JS console afterward: Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefService).resetPrefs();

Comment 11 and comment 16 both assume that I know something about mozmill ("open up the IDE", "view the working editor"). If you're giving steps to reproduce that require interaction with the extension's UI, they need to be in terms of "open the menu labeled X, select option Y", etc. Step by step. Not in terms of big functional descriptions that assume familiarity with the extension...

And the point of comment 15 was trying to narrow down what the problem might be, fwiw. It didn't get far, sadly.

Ok, definitely a regression *on* the 3.0 branch. While 3.0 is working fine 3.0.7 breaks here. I'll track it down now.

This is a regression for 3.0.7 against 3.0.6! The regression window is between the following builds: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009012704 GranParadiso/3.0.7pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7pre) Gecko/2009012804 GranParadiso/3.0.7pre Check-ins: http://tinyurl.com/acnors Moving to Core:Layout for now until this is a core bug.

From the very vague descriptions of the problem in the bug, sounds like perhaps bzbarsky's editor-related checkins could be related. But I haven't tried the steps-to-repro a second time.

Henrik, would you mind also finding the regression range on trunk, so we can look at what checkins fall in both? I'd love reasonable steps to reproduce, still.

I checked the patches which went into 1.9.1 in parallel. The regression is between 08111702 and 08112002. That qualifies bug 445004 as the cause of the problem.

Steps you have to do with MozMill: 1. Install the latest trunk build from Mozmill: http://mozmill.googlecode.com/files/mozmill-trunk-r372.xpi 2. Open MozMill (Cmd+Shift+M) 3. Open the Error Console 4. Evaluate the following command: Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefService).resetPrefs(); With step 4 the editor UI of the MozMill IDE is completely broken. Sorry, but I don't have time to create a separate test for it. We should give this bug a better summary...

I did a quick grep of a substantial subset of addons and didn't see any uses of resetPrefs(). Obviously that wasn't foolproof because I missed the MozMill case, but this doesn't look like a common technique.

Most importantly, STR 1. Open the browser on a clean profile - no extensions at all. 2. Open the error console 3. Put this in the text field: Components.classes["@mozilla.org/preferences-service;1"].getService(Components.interfaces.nsIPrefService).resetPrefs(); 4. Click "Evaluate" The CSS styling will drop from the buttons in the error console ( the All|Errors|Warnings|Clear etc buttons). If you do something that causes a reflow, such as resizing the window, the buttons get redrawn correctly. And more comments from attempting to debug this.... In reply to comment 25: Yeah, I don't think the resetPrefs() call is what's exciting here. I think it's the fact that resetPrefs() causes a reset of large portions of chrome CSS code. Now, probably no extensions are doing that so that is a good thing and we can feel pretty safe about having this on 3.0.x branches. However, it's a core bug, and ought to be fixed on trunk. Now, If you do this on a debug build you get a warning: WARNING: Unable to test style tree integrity -- no content node: file /Users/clint/code/mozcentral/src/layout/base/nsCSSFrameConstructor.cpp, line 9052 And that is getting called because nsPresContext::UpdateAfterPreferencesChanged gets called from a timer that no doubt got set during resetPrefs. I'll keep digging. Here is the stack up to where the NS_WARNING is encountered (from trunk): #0 nsCSSFrameConstructor::ProcessRestyledFrames (this=0x1e92e5b0, aChangeList=@0xbfffdf3c) at /Users/clint/code/mozcentral/src/layout/base/nsCSSFrameConstructor.cpp:9052 #1 0x1312e7cc in nsCSSFrameConstructor::RebuildAllStyleData (this=0x1e92e5b0, aExtraHint=7) at /Users/clint/code/mozcentral/src/layout/base/nsCSSFrameConstructor.cpp:12571 #2 0x13184408 in nsPresContext::RebuildAllStyleData (this=0x1247c00, aExtraHint=7) at /Users/clint/code/mozcentral/src/layout/base/nsPresContext.cpp:1498 #3 0x131853bd in nsPresContext::UpdateAfterPreferencesChanged (this=0x1247c00) at /Users/clint/code/mozcentral/src/layout/base/nsPresContext.cpp:824 #4 0x13185426 in nsPresContext::PrefChangedUpdateTimerCallback (aTimer=0x18159aa0, aClosure=0x1247c00) at /Users/clint/code/mozcentral/src/layout/base/nsPresContext.cpp:142 #5 0x00526582 in nsTimerImpl::Fire (this=0x18159aa0) at /Users/clint/code/mozcentral/src/xpcom/threads/nsTimerImpl.cpp:420 #6 0x005267ba in nsTimerEvent::Run (this=0x18159ae0) at /Users/clint/code/mozcentral/src/xpcom/threads/nsTimerImpl.cpp:512 #7 0x0051fc12 in nsThread::ProcessNextEvent (this=0x8191a0, mayWait=0, result=0xbfffe214) at /Users/clint/code/mozcentral/src/xpcom/threads/nsThread.cpp:510 #8 0x004a87ce in NS_ProcessPendingEvents_P (thread=0x8191a0, timeout=20) at nsThreadUtils.cpp:180 #9 0x11a61889 in nsBaseAppShell::NativeEventCallback (this=0x8495d0) at /Users/clint/code/mozcentral/src/widget/src/xpwidgets/nsBaseAppShell.cpp:121 #10 0x11a17256 in nsAppShell::ProcessGeckoEvents (aInfo=0x8495d0) at /Users/clint/code/mozcentral/src/widget/src/cocoa/nsAppShell.mm:374 #11 0x95dcb5f5 in CFRunLoopRunSpecific () #12 0x95dcbcd8 in CFRunLoopRunInMode () #13 0x93c682c0 in RunCurrentEventLoopInMode () #14 0x93c68012 in ReceiveNextEventCommon () #15 0x93c67f4d in BlockUntilNextEventMatchingListInMode () #16 0x90552d7d in _DPSNextEvent () #17 0x90552630 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] () #18 0x9054b66b in -[NSApplication run] () #19 0x11a15b18 in nsAppShell::Run (this=0x8495d0) at /Users/clint/code/mozcentral/src/widget/src/cocoa/nsAppShell.mm:693 #20 0x1271c44e in nsAppStartup::Run (this=0x863400) at /Users/clint/code/mozcentral/src/toolkit/components/startup/src/nsAppStartup.cpp:192 #21 0x000e6418 in XRE_main (argc=3, argv=0xbffff868, aAppData=0x80e5c0) at /Users/clint/code/mozcentral/src/toolkit/xre/nsAppRunner.cpp:3217 #22 0x000027cb in main (argc=3, argv=0xbffff868) at /Users/clint/code/mozcentral/src/browser/app/nsBrowserApp.cpp:156 And after putting more breakpoints in during the reset prefs call, I see that observers are being notified with the topic "prefservice:after-app-default" which as far as I can tell is only used in BrowserGlue.js??? (http://mxr.mozilla.org/mozilla-central/search?string=prefservice%3Aafter-app-default&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central) I'm not sure if the debugging stuff leads anyone to any clues, but hopefully the non-extension based STR will be a big help.

(In reply to comment #26) > The CSS styling will drop from the buttons in the error console ( the > All|Errors|Warnings|Clear etc buttons). If you do something that causes a > reflow, such as resizing the window, the buttons get redrawn correctly. Interesting. That doesn't happen if the MozMill IDE is open. Thanks for these steps Clint!

OK, using Clint's steps I get a regression range from 2008-12-04-02 to 2008-12-05-02 on trunk. This doesn't match the regression range when bug 445004 landed on trunk. I'm not sure what to make of comment 23, since there are no checkins on the 1.9.1 branch in the range it lists. I guess I'll try testing on trunk with MozMill to see when that regressed...

Using the steps from comment 24, I get a regression range of 2008-11-18-02 to 2008-11-19-05 on trunk. Sort of. The Nov 18 build still changes the editor styling a good bit; it just doesn't drop it completely. Pushlog url: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-11-18+02&enddate=2008-11-19+05 That does in fact include bug 445004. Does MozMill use document.write, by the way?

Oh, and Clint's issue might want a separate bug, of course.

(In reply to comment #29) > That does in fact include bug 445004. Does MozMill use document.write, by the > way? Yes, the edit_area control we're using makes use of document.write.

(In reply to comment #30) > Oh, and Clint's issue might want a separate bug, of course. Bug 480359 filed. Sorry about not doing the regression range, I mistakenly assumed they were the same issue.

OK, the key thing that changed is that the URI for the document.write result document _used_ to be about:blank before the fix for bug 445004 (since the write() was done by chrome code) and is now chrome://mozmill/content/mozmill.html (which is the actual URI of the document the write() happens in). Does the document.written content contain inline stylesheets (using <style>, not <link>)? If it does, then the change to "general.skins.selectedSkin" that the pref service dispatches will trigger the code in nsChromeRegistry::RefreshWindow that tries to reload chrome sheets, and that code will try to load mozmill.html as style data and drop the existing inline sheet...

If that's the problem, by the way, then I bet <style> stylesheets in mozmill.html itself would have been broken both before and after that change. Hence the font size change I observe, I bet.

Oh, and I bet there are all sorts of other preferences whose dynamic changes are handled ... poorly. I would recommend just not using resetPrefs() if you can avoid it, since it will also unset and reset some preferences that code assumes are always set.

I suppose I could fix this on branch by using the DOM API's GetHref, if we need this fixed there... Thoughts?

Oh, and that patch fixes the mozmill.html font resizing too, which was broken all along. On another note, don't use document.write from chrome if you can avoid it. Its behavior _will_ change to follow HTML5 in the near future, and those rules may or may not make sense from chrome. That's not even mentioning the fact that I hope you're sanitizing your strings really carefully...

Comment on attachment 364361 [details] [diff] [review] "Fix" the chrome registry mess FWIW, mozmill should not be using resetprefs... it's a serious bug to use that method in code which is not part of the startup sequence (migrating profiles, mainly)

This bug does not block 1.9.0.8 and, more generally, is not wanted on 1.9.0 based on comment 36 and comment 38.

(In reply to comment #38) > (From update of attachment 364361 [details] [diff] [review]) > FWIW, mozmill should not be using resetprefs... it's a serious bug to use that > method in code which is not part of the startup sequence (migrating profiles, > mainly) Mozmill itself doesn't use it. This came up because Henrik was writing a mozmill test that set and unset a bunch of preferences, and after his test finished running, he thought that he'd get everything back to the default state by calling ResetPrefs. Once he did that, we found all these problems.

I don't think this needs to block given that resetPrefs should not actually be used in this situation.

Comment on attachment 364361 [details] [diff] [review] "Fix" the chrome registry mess sr=dbaron

Pushed http://hg.mozilla.org/mozilla-central/rev/2d87bbd2cf09 I'll work on a 1.9.1 patch, I guess.

Verfied fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090311 Minefield/3.2a1pre ID:20090311030635

I'm not going to worry about fixing this on branches at this point. It's just not worth it.

(In reply to comment #45) > I'm not going to worry about fixing this on branches at this point. It's just > not worth it. I'd agree as this is an edge case, and since 1.9.1 is about to become a security branch, I don't think this is the sort of thing we'd take there anyway.