After Bug 852417, we can enable reader mode on low memory platform. Now, I propose that we introduce a new pref to switch reader mode on general. I think to add these change: * Add the pref to toggle reader mode. * Toggle parse-on-load reader mode. * Toggle "Add reading list" context menu. * Change the name "reader.force_allow" for compatibilities about above changes.

This patch includes: * Toggle parse-on-load reader mode. * Change the name "reader.force_allow" for compatibilities about above changes.

And I'm making patches which adds the pref to toggle "Add reading list" context menu.

I think we need to clean up & make these part smart in the future...

Comment on attachment 745078 [details] [diff] [review] part1: Add the pref to toggle reader mode parsing on load Looks good to me. The pref observer might not be strictly needed, but it's a nice convenience.

Comment on attachment 745078 [details] [diff] [review] part1: Add the pref to toggle reader mode parsing on load > init: function Reader_init() { >+ Services.prefs.addObserver("reader.", this, false); Maybe this should be "reader.parse-on-load." since that's all we look for in observe > uninit: function Reader_uninit() { >+ Services.prefs.removeObserver("reader.", this); Same

(In reply to Mark Finkle (:mfinkle) from comment #5) > Comment on attachment 745078 [details] [diff] [review] > part1: Add the pref to toggle reader mode parsing on load > > Looks good to me. The pref observer might not be strictly needed, but it's a > nice convenience. Thank you review! I intend to reduce the number of times checking the pref by the pref observer. This pref will not change many times. So it's better that caching the value. (In reply to Mark Finkle (:mfinkle) from comment #6) > Comment on attachment 745078 [details] [diff] [review] > part1: Add the pref to toggle reader mode parsing on load > > > init: function Reader_init() { > > >+ Services.prefs.addObserver("reader.", this, false); > > Maybe this should be "reader.parse-on-load." since that's all we look for in > observe > > > uninit: function Reader_uninit() { > >+ Services.prefs.removeObserver("reader.", this); > > Same These points focuses part.3 ( attachment 745635 [details] [diff] [review] ).

I need to think about part 2 and part 3 a bit more. I see disabling the menus as a different issue, and I'm not sure we really need to support it. We don't have prefs for disabling any other menus. Being able to enable/disable the reader-on-pageload is separate (and different) from controlling the context menu as well.

(In reply to Mark Finkle (:mfinkle) from comment #8) > I need to think about part 2 and part 3 a bit more. I see disabling the > menus as a different issue, and I'm not sure we really need to support it. > We don't have prefs for disabling any other menus. Being able to > enable/disable the reader-on-pageload is separate (and different) from > controlling the context menu as well. At first, I had seemed that reader mode are optional features for a current browser. Thus I proposed the patch part 2&3. But I agree your point that is we need to think about it more. I'll rebase part 1, and file a new bug for thinking about part2&3.

Rebase the patch.