Closed Bug 1208744 Opened 9 years ago Closed 8 years ago

PrefCache observer doesn't check pref name, but prefix-matches the pref, causing brokenness if one pref is a strict prefix of another pref

Categories

(Core :: Preferences: Backend, defect)

defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1267868
Tracking Status
firefox44 --- affected

People

(Reporter: Gijs, Unassigned)

Details

STR: 1. add two BoolPrefCaches, for "pref.foo" and "pref.foobar", respectively. 2. change "pref.foobar" to a different value 3. inspect the cache for pref.foo ER: hasn't changed AR: has changed (!) because the observer doesn't actually check that the notification it's getting is for the correct pref. This caught me out in bug 636905 (where I tried to add dom.disable_beforeunload_without_user_interaction or something, when dom.disable_beforeunload already existed), and it was really somewhat lucky that I caught it at all. I think we should fix this gotcha, not least because the result isn't too bad if the 2 prefs are of the same type, but I expect assigning the wrong kind of data into the pref cache is going to make for very sad results.
These are all the prefs that have prefixes in the current combination of firefox.js + all.js, followed by their prefixes: accessibility.browsewithcaret_shortcut.enabled accessibility.browsewithcaret accessibility.tabfocus_applies_to_xul accessibility.tabfocus accessibility.typeaheadfind.autostart accessibility.typeaheadfind app.update.url.override app.update.url browser.cache.use_new_backend_temp browser.cache.use_new_backend browser.gesture.pinch.in.shift browser.gesture.pinch.in browser.gesture.pinch.out.shift browser.gesture.pinch.out browser.link.open_newwindow.disabled_in_fullscreen browser.link.open_newwindow browser.pocket.enabledLocales browser.pocket.enabled browser.safebrowsing.provider.mozilla.lists.mozfull.description browser.safebrowsing.provider.mozilla.lists browser.search.defaultenginename.US browser.search.defaultenginename browser.search.geoSpecificDefaults.url browser.search.geoSpecificDefaults browser.search.update.interval browser.search.update browser.send_pings.max_per_link browser.send_pings browser.sessionstore.privacy_level_deferred browser.sessionstore.privacy_level browser.tabs.crashReporting.emailMe browser.tabs.crashReporting.email browser.tabs.remote.autostart.1 browser.tabs.remote.autostart browser.tabs.warnOnCloseOtherTabs browser.tabs.warnOnClose browser.urlbar.autoFill.typed browser.urlbar.autoFill browser.urlbar.suggest.history.onlyTyped browser.urlbar.suggest.history browser.usedOnWindows10.introURL browser.usedOnWindows10 devtools.browserconsole.filter.networkinfo devtools.browserconsole.filter.network devtools.debugger.log.verbose devtools.debugger.log devtools.netmonitor.har.jsonpCallback devtools.netmonitor.har.jsonp devtools.performance.ui.enable-memory-flame devtools.performance.ui.enable-memory devtools.webconsole.filter.networkinfo devtools.webconsole.filter.network dom.push.adaptive.lastGoodPingInterval.mobile dom.push.adaptive.lastGoodPingInterval dom.push.pingInterval.default dom.push.pingInterval font.weight-override.Avenir-BlackOblique font.weight-override.Avenir-Black font.weight-override.Avenir-BookOblique font.weight-override.Avenir-Book font.weight-override.HelveticaNeue-LightItalic font.weight-override.HelveticaNeue-Light general.smoothScroll.durationToIntervalRatio general.smoothScroll general.smoothScroll.lines.durationMaxMS general.smoothScroll.lines general.smoothScroll.mouseWheel.durationMaxMS general.smoothScroll.mouseWheel general.smoothScroll.other.durationMaxMS general.smoothScroll.other general.smoothScroll.pages.durationMaxMS general.smoothScroll.pages general.smoothScroll.pixels.durationMaxMS general.smoothScroll.pixels general.smoothScroll.scrollbars.durationMaxMS general.smoothScroll.scrollbars intl.hyphenation-alias.bs-* intl.hyphenation-alias.bs intl.hyphenation-alias.de-* intl.hyphenation-alias.de intl.hyphenation-alias.en-* intl.hyphenation-alias.en intl.hyphenation-alias.no-* intl.hyphenation-alias.no intl.hyphenation-alias.sr-* intl.hyphenation-alias.sr intl.imm.composition_font.japanist_2003 intl.imm.composition_font javascript.options.compact_on_user_inactive_delay javascript.options.compact_on_user_inactive javascript.options.ion.offthread_compilation javascript.options.ion javascript.options.mem.gc_incremental_slice_ms javascript.options.mem.gc_incremental javascript.options.strict.debug javascript.options.strict layers.dump-client-layers layers.dump layout.frame_rate.precise layout.frame_rate layout.imagevisibility.enabled_for_browser_elements_only layout.imagevisibility.enabled media.getusermedia.aec_enabled media.getusermedia.aec media.getusermedia.agc_enabled media.getusermedia.agc media.getusermedia.noise_enabled media.getusermedia.noise media.gmp-manager.url.override media.gmp-manager.url media.navigator.load_adapt.avg_seconds media.navigator.load_adapt media.peerconnection.ice.tcp_so_sock_count media.peerconnection.ice.tcp mousewheel.default.action.override_x mousewheel.default.action mousewheel.with_alt.action.override_x mousewheel.with_alt.action mousewheel.with_control.action.override_x mousewheel.with_control.action mousewheel.with_meta.action.override_x mousewheel.with_meta.action mousewheel.with_shift.action.override_x mousewheel.with_shift.action mousewheel.with_win.action.override_x mousewheel.with_win.action network.IDN.whitelist.cat network.IDN.whitelist.ca network.auth.force-generic-ntlm-v1 network.auth.force-generic-ntlm network.dnsCacheExpirationGracePeriod network.dnsCacheExpiration network.http.accept-encoding.secure network.http.accept-encoding network.http.pipelining.abtest network.http.pipelining network.http.spdy.enabled.deps network.http.spdy.enabled network.protocol-handler.external.disks network.protocol-handler.external.disk network.protocol-handler.external.ttps network.protocol-handler.external.ttp network.proxy.ftp_port network.proxy.ftp network.proxy.http_port network.proxy.http network.proxy.socks_port network.proxy.socks network.proxy.ssl_port network.proxy.ssl nglayout.debug.paint_flashing_chrome nglayout.debug.paint_flashing plugin.state.nprobloxproxy plugin.state.nproblox plugin.state.playerplugin.charter plugin.state.playerplugin reader.color_scheme.values reader.color_scheme security.sandbox.windows.log.stackTraceDepth security.sandbox.windows.log services.sync.prefs.sync.accessibility.typeaheadfind.linksonly services.sync.prefs.sync.accessibility.typeaheadfind signon.rememberSignons.visibilityToggle signon.rememberSignons toolkit.telemetry.server_owner toolkit.telemetry.server ui.key.menuAccessKeyFocuses ui.key.menuAccessKey I haven't audited which of these, if any, use prefcaches, but it seems only a matter of time until this bites us again, also because it seems we create these in the DOM bindings code, amongst other places, purely from codegen. In fact, a casual look through: https://dxr.mozilla.org/mozilla-central/search?q=regexp%3A%3A%3Aadd\S%2Bcache&redirect=false&case=false shows that we are already having this problem in practice: https://dxr.mozilla.org/mozilla-central/rev/b68eab795f9de072bee12821b0f09422e5aa0da9/layout/base/nsPresShell.cpp#5858 which was added in bug 930535. :tn, do you have time to investigate fixing this?
Flags: needinfo?(tnikkel)
(In reply to :Gijs Kruitbosch from comment #1) > layout.imagevisibility.enabled_for_browser_elements_only > layout.imagevisibility.enabled > > shows that we are already having this problem in practice: > > https://dxr.mozilla.org/mozilla-central/rev/ > b68eab795f9de072bee12821b0f09422e5aa0da9/layout/base/nsPresShell.cpp#5858 > > which was added in bug 930535. > > :tn, do you have time to investigate fixing this? layout.imagevisibility.enabled_for_browser_elements_only is obsolete, so I'll just remove it: bug 1213953.
Flags: needinfo?(tnikkel)
How about adding "exact match" mode to pref callbacks?
Did bug 1267868 address this?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Kit Cambridge [:kitcambridge] from comment #4) > Did bug 1267868 address this? Looks like it, yes! \o/ Thanks for flagging that up.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(gijskruitbosch+bugs)
Resolution: --- → DUPLICATE
Cool! I wasn't sure if that was the same issue at first, so that's why I didn't dupe it. :-)
You need to log in before you can comment on or make changes to this bug.