Some check-in today caused the applications pref pane to become completely empty. Probably bug 408957, which also caused bug 410894. Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b3pre) Gecko/2008010504 Minefield/3.0b3pre
Even if JS2/ES4 makes the switch body-block scope let declarations, that's not what this code (or the downloads.js code) wants -- this code wants implicit blocks around switch-case statement lists. But due to fall-through without break, these cannot be scoped without analyzing control flow. More sane but still questionable would be to scope all let decls in all case code to the switch's body block, which is what JS1.7 did. But there's always explicit block containing let decls, or let blocks if putting the decls in the head (with the ability to use outer synonyms in initializers) is important. It's up to myk, but my 2 cents favor keeping let but using explicit let blocks. /be
Created attachment 295570 [details] [diff] [review] use explicit/let blocks
Comment on attachment 295570 [details] [diff] [review] use explicit/let blocks I agree with Brendan; this is the right fix. r=myk
Attachment #295570 - Flags: review?(myk) → review+
Attachment #295570 - Flags: approval1.9?
Created attachment 295748 [details] [diff] [review] Simple alive test
Attachment #295748 - Flags: review?(myk)
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Comment on attachment 295748 [details] [diff] [review] Simple alive test Thanks for adding a test for this! >diff --git a/browser/components/preferences/Makefile.in b/browser/components/preferences/Makefile.in >+ifdef MOZ_MOCHITEST >+DIRS += tests >+endif Nit: it seems like other browser chrome mochitests have been using "test" singular as their directory name, so perhaps this should do the same. On the other hand, lots of other tests are using the plural version, so it probably doesn't matter. >diff --git a/browser/components/preferences/tests/Makefile.in b/browser/components/preferences/tests/Makefile.in >+libs:: $(_BROWSER_FILES) >+ $(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/$(relativesrcdir) Despite the apparent duplication, this should be: + $(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/browser/$(relativesrcdir) Which puts the tests in: _tests/testing/mochitest/browser/browser/components/preferences So that you can run them from the command line via: perl runtests.pl --browser-chrome --test-path=browser/components/preferences r=myk with this change >diff --git a/browser/components/preferences/tests/browser_bug410900.js b/browser/components/preferences/tests/browser_bug410900.js >+ var doc = win && win.document; Why isn't this just "var doc = win.document"?
Attachment #295748 - Flags: review?(myk) → review+
(In reply to comment #8) > >+ var doc = win && win.document; > > Why isn't this just "var doc = win.document"? > Just a leftover from a previous iteration - I removed this and corrected the nsinstall path on checkin.
Flags: in-testsuite? → in-testsuite+
I disabled the last part of this test due to a failure that appeared after bug 397815 landed. chrome://mochikit/content/browser/browser/components/preferences/tests/browser_bug410900.js PASS - Pref window opened PASS - Specified pane was opened PASS - handlersView is present FAIL - App handler list populated - chrome://mochikit/content/browser/browser/components/preferences/tests/browser_bug410900.js I downloaded an hourly to check this, and the applications pane looked fine, so I'm guessing it's a problem with the test itself?
(In reply to comment #10) > I disabled the last part of this test due to a failure that appeared after bug > 397815 landed. I don't see any way that that patch could have affected this test. It might have just been a fluke.
(A fluke that that test failed at the same time that that patch was checked in, I mean. The test itself could have failed for a number of reasons.)
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b3) Gecko/2008020514 Firefox/3.0b3 ID:2008020514 - > Verified fixed
Status: RESOLVED → VERIFIED
I have this problem in Firefox 3 Beta 5 on Xubuntu, with settings imported from my previous installation of Firefox 2 on LinuxMint 4. It is annoying.
The test failed again TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/preferences/tests/browser_bug410900.js | handlersView is present TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/preferences/tests/browser_bug410900.js | App handler list populated Using timeout to see if window is opened and the document for it is loaded is error prone. Would be better if load event could be used.
I still have the same problem, in FireFox 3.0 (rv.1.9. What's that?) on Xubuntu.
You need to log in before you can comment on or make changes to this bug.