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)
Core
Preferences: Backend
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.
Reporter | ||
Comment 1•9 years ago
|
||
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)
Comment 2•9 years ago
|
||
(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)
Comment 3•9 years ago
|
||
How about adding "exact match" mode to pref callbacks?
Reporter | ||
Comment 5•8 years ago
|
||
(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
Comment 6•8 years ago
|
||
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.
Description
•