Closed Bug 1363118 Opened 4 years ago Closed 4 years ago

Rename browser.shell.skipDefaultBrowserCheck pref for clarity

Categories

(Firefox :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: dao, Assigned: dao)

Details

(Whiteboard: [bugday-20170531])

Attachments

(1 file)

Followup from bug 1322723 comment 4.
Oops, the migration code is wrong it seems.
Comment on attachment 8865559 [details]
Bug 1363118 - Rename browser.shell.skipDefaultBrowserCheck pref for clarity.

https://reviewboard.mozilla.org/r/137174/#review140310

::: browser/app/profile/firefox.js:239
(Diff revision 2)
>  pref("browser.shell.skipDefaultBrowserCheckOnFirstRun", true);
>  #endif
> -pref("browser.shell.skipDefaultBrowserCheck", true);
> +pref("browser.shell.defaultBrowserCheckSkippedOnFirstRun", false);

I... honestly don't understand what the difference is between "skipDefaultBrowserCheckOnFirstRun" and "defaultBrowserCheckSkippedOnFirstRun" on first read.

Can we use "didSkipDefaultBrowserCheckOnFirstRun" or "alreadySkippedDefaultBrowserCheck" instead?
Attachment #8865559 - Flags: review?(gijskruitbosch+bugs)
(In reply to :Gijs from comment #4)
> Comment on attachment 8865559 [details]
> Bug 1363118 - Rename browser.shell.skipDefaultBrowserCheck pref for clarity.
> 
> https://reviewboard.mozilla.org/r/137174/#review140310
> 
> ::: browser/app/profile/firefox.js:239
> (Diff revision 2)
> >  pref("browser.shell.skipDefaultBrowserCheckOnFirstRun", true);
> >  #endif
> > -pref("browser.shell.skipDefaultBrowserCheck", true);
> > +pref("browser.shell.defaultBrowserCheckSkippedOnFirstRun", false);
> 
> I... honestly don't understand what the difference is between
> "skipDefaultBrowserCheckOnFirstRun" and
> "defaultBrowserCheckSkippedOnFirstRun" on first read.

Skip vs. skipped basically. Better on second read?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Dão Gottwald [::dao] from comment #5)
> (In reply to :Gijs from comment #4)
> > Comment on attachment 8865559 [details]
> > Bug 1363118 - Rename browser.shell.skipDefaultBrowserCheck pref for clarity.
> > 
> > https://reviewboard.mozilla.org/r/137174/#review140310
> > 
> > ::: browser/app/profile/firefox.js:239
> > (Diff revision 2)
> > >  pref("browser.shell.skipDefaultBrowserCheckOnFirstRun", true);
> > >  #endif
> > > -pref("browser.shell.skipDefaultBrowserCheck", true);
> > > +pref("browser.shell.defaultBrowserCheckSkippedOnFirstRun", false);
> > 
> > I... honestly don't understand what the difference is between
> > "skipDefaultBrowserCheckOnFirstRun" and
> > "defaultBrowserCheckSkippedOnFirstRun" on first read.
> 
> Skip vs. skipped basically. Better on second read?

I mean, after I read all the context in the other bug, sure. I just think it'd be better to err on the side of as much past tense as possible. Maybe "defaultBrowserCheckWasSkippedOnFirstRun" ?
Flags: needinfo?(gijskruitbosch+bugs)
Comment on attachment 8865559 [details]
Bug 1363118 - Rename browser.shell.skipDefaultBrowserCheck pref for clarity.

https://reviewboard.mozilla.org/r/137174/#review140602
Attachment #8865559 - Flags: review?(gijskruitbosch+bugs) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/553198d4b70f
Rename browser.shell.skipDefaultBrowserCheck pref for clarity. r=Gijs
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/38346bf36faa
Rename browser.shell.skipDefaultBrowserCheck pref for clarity. r=Gijs
Flags: needinfo?(dao+bmo)
Backed out for failing xpcshell's test_browserGlue_urlbar_defaultbehavior_migration.js and test_browserGlue_migration_loop_cleanup.js:

https://hg.mozilla.org/integration/autoland/rev/86504695e75226ab066c183c844014a7274b1358

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=38346bf36faaff4fa4d99ba88d66475f8a18ac57&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=97695182&repo=autoland

[task 2017-05-09T16:23:26.778974Z] 16:23:26     INFO -  TEST-START | browser/components/places/tests/unit/test_browserGlue_urlbar_defaultbehavior_migration.js
[task 2017-05-09T16:23:28.446967Z] 16:23:28  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/places/tests/unit/test_browserGlue_urlbar_defaultbehavior_migration.js | xpcshell return code: 0
[task 2017-05-09T16:23:28.458155Z] 16:23:28     INFO -  TEST-INFO took 1671ms
[task 2017-05-09T16:23:28.458474Z] 16:23:28     INFO -  >>>>>>>
[task 2017-05-09T16:23:28.460399Z] 16:23:28     INFO -  PID 7220 | [7220] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /home/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2888
[task 2017-05-09T16:23:28.460720Z] 16:23:28     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2017-05-09T16:23:28.460973Z] 16:23:28     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2017-05-09T16:23:28.463553Z] 16:23:28     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2017-05-09T16:23:28.465662Z] 16:23:28     INFO -  running event loop
[task 2017-05-09T16:23:28.468726Z] 16:23:28     INFO -  browser/components/places/tests/unit/test_browserGlue_urlbar_defaultbehavior_migration.js | Starting
[task 2017-05-09T16:23:28.474438Z] 16:23:28     INFO -  (xpcshell/head.js) | test pending (2)
[task 2017-05-09T16:23:28.476940Z] 16:23:28     INFO -  "Migrate default.behavior = 0"
[task 2017-05-09T16:23:28.479193Z] 16:23:28     INFO -  PID 7220 | JavaScript error: jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js, line 1968: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]
[task 2017-05-09T16:23:28.481217Z] 16:23:28     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2017-05-09T16:23:28.483288Z] 16:23:28     INFO -  Unexpected exception NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]
[task 2017-05-09T16:23:28.485383Z] 16:23:28     INFO -  BG__migrateUI@jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js:1968:35
[task 2017-05-09T16:23:28.487327Z] 16:23:28     INFO -  BG_observe@jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js:397:11
[task 2017-05-09T16:23:28.490487Z] 16:23:28     INFO -  setupBehaviorAndMigrate@/home/worker/workspace/build/tests/xpcshell/tests/browser/components/places/tests/unit/test_browserGlue_urlbar_defaultbehavior_migration.js:37:3
[task 2017-05-09T16:23:28.492606Z] 16:23:28     INFO -  @/home/worker/workspace/build/tests/xpcshell/tests/browser/components/places/tests/unit/test_browserGlue_urlbar_defaultbehavior_migration.js:42:3
[task 2017-05-09T16:23:28.494923Z] 16:23:28     INFO -  _run_next_test@/home/worker/workspace/build/tests/xpcshell/head.js:1554:9
[task 2017-05-09T16:23:28.497003Z] 16:23:28     INFO -  run@/home/worker/workspace/build/tests/xpcshell/head.js:703:9
[task 2017-05-09T16:23:28.499138Z] 16:23:28     INFO -  _do_main@/home/worker/workspace/build/tests/xpcshell/head.js:211:5
[task 2017-05-09T16:23:28.502580Z] 16:23:28     INFO -  _execute_test@/home/worker/workspace/build/tests/xpcshell/head.js:542:5
[task 2017-05-09T16:23:28.504398Z] 16:23:28     INFO -  @-e:1:1
[task 2017-05-09T16:23:28.506250Z] 16:23:28     INFO -  exiting test
[task 2017-05-09T16:23:28.508677Z] 16:23:28     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]" {file: "jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js" line: 1968}]"
[task 2017-05-09T16:23:28.510963Z] 16:23:28     INFO -  PID 7220 | [7220] WARNING: NS_ENSURE_TRUE(aObserver) failed: file /home/worker/workspace/build/src/modules/libpref/nsPrefBranch.cpp, line 748
[task 2017-05-09T16:23:28.514728Z] 16:23:28     INFO -  PID 7220 | [7220] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /home/worker/workspace/build/src/modules/libpref/Preferences.cpp, line 1721
[task 2017-05-09T16:23:28.516761Z] 16:23:28     INFO -  PID 7220 | [7220] WARNING: '!compMgr', file /home/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp, line 63
[task 2017-05-09T16:23:28.518895Z] 16:23:28     INFO -  PID 7220 | [7220] WARNING: OOPDeinit() without successful OOPInit(): file /home/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 3601
[task 2017-05-09T16:23:28.522323Z] 16:23:28     INFO -  PID 7220 | nsStringStats
[task 2017-05-09T16:23:28.524242Z] 16:23:28     INFO -  PID 7220 |  => mAllocCount:           9430
[task 2017-05-09T16:23:28.526150Z] 16:23:28     INFO -  PID 7220 |  => mReallocCount:          153
[task 2017-05-09T16:23:28.529176Z] 16:23:28     INFO -  PID 7220 |  => mFreeCount:            9430
[task 2017-05-09T16:23:28.531131Z] 16:23:28     INFO -  PID 7220 |  => mShareCount:           7035
[task 2017-05-09T16:23:28.533035Z] 16:23:28     INFO -  PID 7220 |  => mAdoptCount:            497
[task 2017-05-09T16:23:28.536009Z] 16:23:28     INFO -  PID 7220 |  => mAdoptFreeCount:        497
[task 2017-05-09T16:23:28.537975Z] 16:23:28     INFO -  PID 7220 |  => Process ID: 7220, Thread ID: 139767380888576
[task 2017-05-09T16:23:28.539913Z] 16:23:28     INFO -  <<<<<<<
[task 2017-05-09T16:23:28.562915Z] 16:23:28     INFO -  TEST-START | browser/components/tests/unit/test_browserGlue_migration_loop_cleanup.js
[task 2017-05-09T16:23:29.979281Z] 16:23:29  WARNING -  TEST-UNEXPECTED-FAIL | browser/components/tests/unit/test_browserGlue_migration_loop_cleanup.js | xpcshell return code: 0
[task 2017-05-09T16:23:29.979682Z] 16:23:29     INFO -  TEST-INFO took 1418ms
[task 2017-05-09T16:23:29.982421Z] 16:23:29     INFO -  >>>>>>>
[task 2017-05-09T16:23:29.986648Z] 16:23:29     INFO -  PID 7243 | [7243] WARNING: Couldn't get the user appdata directory. Crash events may not be produced.: file /home/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 2888
[task 2017-05-09T16:23:29.988627Z] 16:23:29     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2017-05-09T16:23:29.990567Z] 16:23:29     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2017-05-09T16:23:29.992519Z] 16:23:29     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2017-05-09T16:23:29.994414Z] 16:23:29     INFO -  running event loop
[task 2017-05-09T16:23:30.000163Z] 16:23:29     INFO -  browser/components/tests/unit/test_browserGlue_migration_loop_cleanup.js | Starting test_check_cleanup_loop_prefs
[task 2017-05-09T16:23:30.002153Z] 16:23:30     INFO -  (xpcshell/head.js) | test test_check_cleanup_loop_prefs pending (2)
[task 2017-05-09T16:23:30.004461Z] 16:23:30     INFO -  PID 7243 | JavaScript error: jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js, line 1968: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]
[task 2017-05-09T16:23:30.006654Z] 16:23:30     INFO -  (xpcshell/head.js) | test run_next_test 0 finished (2)
[task 2017-05-09T16:23:30.008737Z] 16:23:30     INFO -  Unexpected exception NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]
[task 2017-05-09T16:23:30.010787Z] 16:23:30     INFO -  BG__migrateUI@jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js:1968:35
[task 2017-05-09T16:23:30.012867Z] 16:23:30     INFO -  BG_observe@jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js:397:11
[task 2017-05-09T16:23:30.015781Z] 16:23:30     INFO -  test_check_cleanup_loop_prefs@/home/worker/workspace/build/tests/xpcshell/tests/browser/components/tests/unit/test_browserGlue_migration_loop_cleanup.js:16:3
[task 2017-05-09T16:23:30.017770Z] 16:23:30     INFO -  _run_next_test@/home/worker/workspace/build/tests/xpcshell/head.js:1554:9
[task 2017-05-09T16:23:30.021588Z] 16:23:30     INFO -  run@/home/worker/workspace/build/tests/xpcshell/head.js:703:9
[task 2017-05-09T16:23:30.023669Z] 16:23:30     INFO -  _do_main@/home/worker/workspace/build/tests/xpcshell/head.js:211:5
[task 2017-05-09T16:23:30.025580Z] 16:23:30     INFO -  _execute_test@/home/worker/workspace/build/tests/xpcshell/head.js:542:5
[task 2017-05-09T16:23:30.027616Z] 16:23:30     INFO -  @-e:1:1
[task 2017-05-09T16:23:30.029688Z] 16:23:30     INFO -  exiting test
[task 2017-05-09T16:23:30.034914Z] 16:23:30     INFO -  "CONSOLE_MESSAGE: (error) [JavaScript Error: "NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]" {file: "jar:file:///home/worker/workspace/build/application/firefox/browser/omni.ja!/components/nsBrowserGlue.js" line: 1968}]"
[task 2017-05-09T16:23:30.037020Z] 16:23:30     INFO -  PID 7243 | [7243] WARNING: NS_ENSURE_TRUE(aObserver) failed: file /home/worker/workspace/build/src/modules/libpref/nsPrefBranch.cpp, line 748
[task 2017-05-09T16:23:30.039167Z] 16:23:30     INFO -  PID 7243 | [7243] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80070057: file /home/worker/workspace/build/src/modules/libpref/Preferences.cpp, line 1721
[task 2017-05-09T16:23:30.041166Z] 16:23:30     INFO -  PID 7243 | [7243] WARNING: '!compMgr', file /home/worker/workspace/build/src/xpcom/components/nsComponentManagerUtils.cpp, line 63
[task 2017-05-09T16:23:30.043251Z] 16:23:30     INFO -  PID 7243 | [7243] WARNING: OOPDeinit() without successful OOPInit(): file /home/worker/workspace/build/src/toolkit/crashreporter/nsExceptionHandler.cpp, line 3601
[task 2017-05-09T16:23:30.046250Z] 16:23:30     INFO -  PID 7243 | nsStringStats
[task 2017-05-09T16:23:30.048176Z] 16:23:30     INFO -  PID 7243 |  => mAllocCount:           8956
[task 2017-05-09T16:23:30.050234Z] 16:23:30     INFO -  PID 7243 |  => mReallocCount:          143
[task 2017-05-09T16:23:30.054387Z] 16:23:30     INFO -  PID 7243 |  => mFreeCount:            8956
[task 2017-05-09T16:23:30.056312Z] 16:23:30     INFO -  PID 7243 |  => mShareCount:           6678
[task 2017-05-09T16:23:30.058200Z] 16:23:30     INFO -  PID 7243 |  => mAdoptCount:            315
[task 2017-05-09T16:23:30.060075Z] 16:23:30     INFO -  PID 7243 |  => mAdoptFreeCount:        315
[task 2017-05-09T16:23:30.061971Z] 16:23:30     INFO -  PID 7243 |  => Process ID: 7243, Thread ID: 139992163576832
[task 2017-05-09T16:23:30.063958Z] 16:23:30     INFO -  <<<<<<<
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/779a202b73af
Rename browser.shell.skipDefaultBrowserCheck pref for clarity. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/779a202b73af
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I have reproduced this bug with Nightly 55.0a1 (2017-05-08) on Windows 10, 64 bit!

The fix is now verified on latest Nightly!

Build ID 	20170531030204
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Whiteboard: [bugday-20170531]
You need to log in before you can comment on or make changes to this bug.