Closed Bug 629038 Opened 9 years ago Closed 9 years ago

Disabling reflow zoom requires a browser restart

Categories

(Firefox for Android Graveyard :: Panning/Zooming, defect, P2, minor)

Tracking

(fennec2.0b5+)

VERIFIED FIXED
Firefox 4.0
Tracking Status
fennec 2.0b5+ ---

People

(Reporter: mbrubeck, Assigned: vingtetun)

References

Details

Attachments

(1 file, 2 obsolete files)

Steps to reproduce:
1. Enable "reformat text on zoom" in the Fennec preference pane.
2. Disable "reformat text on zoom".
3. Double-tap to zoom in web content.

Expected results: Content is not reformatted.
Actual results: Content is reformatted until the next browser restart.

about:config shows that the pref is correctly set to "false", but getBoolPref in the content script still returns true.  Maybe a problem with our child process preferences cache.
tracking-fennec: --- → ?
tracking-fennec: ? → 2.0b5+
Assignee: mbrubeck → doug.turner
Attached patch Patch (obsolete) — Splinter Review
My understanding of the bug is this is a regression from bug 616414. Afaict there is 4 cases for preferences:
 1. the pref exist in the default branch and is set to a custom value
 2. the pref exist in the default branch and is set to the original value
 3. the pref does not exist in the default branch and is set to a custom value
 4. the pref does not exist in the default branch and is reset (clearUserPref)

We're handling case 1,3,4 but case 2 actually act as case 4.

Sorry doug, I don't want to steal you patch I was just curious to understand what this is not working (and so I have finished by doing a patch...)
Attachment #509467 - Flags: feedback?(doug.turner)
Comment on attachment 509467 [details] [diff] [review]
Patch

the general idea is to protect against an invalid pref. 

I think you can early return if prefHasValue == false.

+        nsCOMPtr<nsIPrefService> prefsService = do_GetService(NS_PREFSERVICE_CONTRACTID);

Can't you use |prefService|?


+        NS_ConvertUTF16toUTF8 prefName(aData);

strData is defined above.  How about change that to UTF8 and reusing it?
Attachment #509467 - Flags: feedback?(doug.turner) → feedback+
Assignee: doug.turner → 21
Attached patch Patch (obsolete) — Splinter Review
(In reply to comment #2)
> +        nsCOMPtr<nsIPrefService> prefsService =
> do_GetService(NS_PREFSERVICE_CONTRACTID);
> 
> Can't you use |prefService|?
>

I need GetDefaultBranch which is not available on nsIPrefServiceInternal
Attachment #509467 - Attachment is obsolete: true
Attachment #511389 - Flags: review?(dwitte)
can't you just QI from nsIPrefServiceInternal to nsIPrefService
Attached patch Patch v0.2Splinter Review
done.
Attachment #511389 - Attachment is obsolete: true
Attachment #511427 - Flags: review?(doug.turner)
Attachment #511389 - Flags: review?(dwitte)
Note for testing: Other Fennec Prefs in the lists below are also affected by this patch.

vingtetun: tchung: the synchronisation of preferences between the chrome process and the content process is broken
[09:40am] tchung: vingtetun: i see. so does the fix affect other prefs, or just reflow zoom?
[09:41am] vingtetun: it affect other prefs too
[09:42am] tchung: vingtetun: which other existing prefs? 
[09:44am] vingtetun: all the prefs that pre-exists in firefox (all the pref from all.js and mobile.js)
[09:44am] vingtetun: http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/init/all.js
[09:45am] vingtetun: http://hg.mozilla.org/mobile-browser/file/40de821d2c78/app/mobile.js
Whiteboard: [has-patch]
Attachment #511427 - Flags: review?(doug.turner) → review+
Keywords: checkin-needed
Whiteboard: [has-patch]
http://hg.mozilla.org/mozilla-central/rev/6b6e30659dab
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → fennec2.0
verified FIXED on build:

Mozilla/5.0 (Android; Linux armv71; rv:2.0b12pre) Gecko/20110216 Firefox/4.0b12pre Fennec/4.0b5pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.