Closed Bug 1655362 Opened 4 years ago Closed 3 years ago

Default Zoom support for SeaMonkey

Categories

(SeaMonkey :: General, enhancement)

enhancement

Tracking

(seamonkey2.53+ fixed, seamonkey2.57esr? affected)

RESOLVED FIXED
seamonkey 2.83
Tracking Status
seamonkey2.53 + fixed
seamonkey2.57esr ? affected

People

(Reporter: buc, Assigned: buc)

References

Details

(Whiteboard: SM2.53.5)

Attachments

(4 files, 8 obsolete files)

16.33 KB, patch
Details | Diff | Splinter Review
7.50 KB, patch
frg
: review+
Details | Diff | Splinter Review
7.51 KB, patch
frg
: review+
Details | Diff | Splinter Review
7.44 KB, patch
frg
: review+
Details | Diff | Splinter Review

Default zoom is a long-awaited feature for Mozilla browsers.

Unlike some things like layout.css.devPixelsPerPx, GDK_SCALE etc. it allows users to zoom "content only", without interface distortion. This way a comfortable viewing can be provided on large or HiDPI displays.

The site-specific values can be used as usual (Ctrl+, Ctrl-), but the global default can be changed now to be not 1.0 only.

The internal code seems ready for it for years (including SeaMonkey). Only UI is missing. Firefox has already added such an UI since FF73 (as one of changes from bug #1590485).

There is a hackish work-around, by use of BrowserConsole input string (when toggled devtools.chrome.enabled). For example, to set 120% zoom:

FullZoom.contentPrefs.setGlobal(FullZoom.name,1.2,getBrowser().docShell);

to come back:

FullZoom.contentPrefs.removeGlobal(FullZoom.name, getBrowser().docShell);

Another way is the use of zoom.minPercent (or zoom.maxPercent) preference, but then only one direction of changes is allowed (for site-specific values).

There were various rather cumbersome extensions as well.

Support of default zoom looks like easy implemented and adds power feature for modern SeaMonkey users.

Preliminary patch.

Add "Default zoom" field into the group of "Zoom options" in Preferences-->Appearance-->Content , with respect for zoom ranges.

Some notes:

The field is at the bottom of the group, otherwise it creates aestetic issues for me. :)

The code should be prepared by someone more familiar with javascript and Mozilla internals.
(Probably some reorganisation with the ranges stuff is needed as well, note max/min naming "inversion" etc.)

When testing, make sure you have not set individual site-specific zoom for the test page (ie. Ctrl+/Ctrl-), or just use Ctrl0 to reset anyway.

Any site-specific zoom, once set (even to be equal to the current default), does not affected by default zoom changes, until you reset it by Ctrl0.

There is no need to recompile for tests, if you can apply the changes into omni.ja internals.

add handleCompletion() to avoid js exceptions in some cases.

Attachment #9166178 - Attachment is obsolete: true

2.57 and up version. increment="10" in the numberbox does no longer work but I think this is because of some mozilla "improvements" and the move of spinbox to common. Should be a follow-up I think. But I am not sure this is 100% correct contentservices2 is async only I think so might only work my chance.

Attachment #9167108 - Flags: review?(iann_bugzilla)
Attachment #9167108 - Flags: approval-comm-esr60?

[Approval Request Comment]
Regression caused by (bug #): --
User impact if declined: no default zoom
Testing completed (on m-c, etc.): 2.53.5
Risk to taking this patch (and alternatives if risky): minor
String changes made by this patch: one string can be take from Fx.

Assignee: nobody → frgrahl
Attachment #9166430 - Attachment is obsolete: true
Attachment #9167109 - Flags: approval-comm-release?

In tomorrows 2.53.5 unofficial if someone want to test it.

Assignee: frgrahl → dmitry
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Version: SeaMonkey 2.53 Branch → Trunk
See Also: → 1656316
Whiteboard: SM2.53.5

Comment on attachment 9167108 [details] [diff] [review]
1655362-default-zoom-v2-257.patch

At the moment, if you have displayed the zoom status in the status bar, then it does not get updated when you change the default zoom. You probably need to add an extra case to the observe for the pref browser.content.full-zoom in https://searchfox.org/comm-central/source/suite/browser/navigator.js#87-89
f+ for the moment

Attachment #9167108 - Flags: review?(iann_bugzilla) → feedback+

it does not get updated when you change the default zoom.

Already seen something similar -- if you start to change default zoom in preferences pane, the status bar at the (background) window updates with a "one step lag".

The patch attached fixes the issue. (Not sure whether it is a kludge or not...)

Looks like some ordering issue -- when updateZoomStatus() is called, ZoomManager.zoom still has the old value.

Flags: needinfo?(iann_bugzilla)

Probably final correct variant to fix zoomstatus.

First of all, it seems yet another bug found, regarding onLocationChange():

In this fragment:

  onLocationChange: function(aURI)
  {
    // Make sure zoom values loaded before updating
    window.setTimeout(updateZoomStatus(), 0);
  }

there is an extra "()", which seems prevent setTimeout() to take any effect. This code should be:

  onLocationChange: function(aURI)
  {
    // Make sure zoom values loaded before updating
    window.setTimeout(updateZoomStatus, 0);
  }

Probably setTimeout() is not needed here at all. Even though it doesn't turn on, everything ends up working as it should and no one complains...
Thus I have dropped setTimeout() and use just updateZoomStatus() for onLocationChange() .

OTOH for the fixing the initial zoomstatus issue, setTimeout() way helps. We just need to use it "around" updateZoomStatus() for onContentPrefSet()/onContentPrefRemoved() observers. After that, no more lagging in zoomstatus when update default zoom in prefs.

Attachment #9169000 - Attachment is obsolete: true
Flags: needinfo?(iann_bugzilla) → needinfo?(frgrahl)
Flags: needinfo?(iann_bugzilla)
Attached file 1655362-default-zoom-v3-253x.patch (obsolete) —

v3 seems to work.

Attachment #9167109 - Attachment is obsolete: true
Attachment #9170802 - Attachment is obsolete: true
Attachment #9167109 - Flags: approval-comm-release?
Flags: needinfo?(frgrahl)
Attachment #9171587 - Flags: feedback?(iann_bugzilla)

Comment on attachment 9167108 [details] [diff] [review]
1655362-default-zoom-v2-257.patch

needs to be updated

Attachment #9167108 - Flags: approval-comm-esr60?

Updated patches in Bills 2.53.5b1 pre and 2.57 builds. Seem to work fine.

Since cps2.getGlobal() is async, it is better to correct code a bit:

--- pref-content.js     2010-01-01 00:00:00.000000000 +0300
+++ pref-content.js-OK  2020-10-17 22:05:42.191371626 +0300
@@ -41,25 +41,18 @@

   let cps2 = Services.contentPrefs.QueryInterface(Ci.nsIContentPrefService2);

-  let value = undefined;
-
   var zoomValue = cps2.getCachedGlobal("browser.content.full-zoom", null);
   if (zoomValue && zoomValue.value)
-    value = zoomValue.value;
+    defaultElement.value = Math.round(zoomValue.value * 100);
   else {
+    defaultElement.value = 100;
     cps2.getGlobal("browser.content.full-zoom", null, {
       handleResult(pref) {
-        value = pref.value;
+        defaultElement.value = Math.round(pref.value * 100);
       },
       handleCompletion(reason) {}
     });
   }
-
-  if (value === undefined) {
-    value = 1.0
-  }
-
-  defaultElement.value = Math.round(value * 100);
 }

 /**
Flags: needinfo?(frgrahl)

l10n strings for SM supported locales. Derived from the latest Firefox code.

Some "accesskey" are changed (since already used), in some most obvious ways.

New version incorporating the changes and a few else patch removals. Bogged down with backports. Will test soon.

Attachment #9167108 - Attachment is obsolete: true
Attachment #9171587 - Attachment is obsolete: true
Attachment #9171587 - Flags: feedback?(iann_bugzilla)
Flags: needinfo?(frgrahl)

Rebased comm central patch. r+ from me.

Attachment #9190100 - Flags: review+

Rebased 2.57 patch. The 2.57 forked spinbox code no longer supports the 10 step. Left it in for a followup bug. This affectes other spinboxes too.

Attachment #9190101 - Flags: review+
Attachment #9190101 - Flags: approval-comm-esr60+

No changes from previous code level. Checkin comment updated and r/a+ from me. Thanks

Attachment #9185024 - Attachment is obsolete: true
Attachment #9190102 - Flags: review+
Attachment #9190102 - Flags: approval-comm-release+

Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/7aabdf38fd05
Default Zoom support for SeaMonkey. r=frg

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Future
Target Milestone: Future → seamonkey 2.82
Flags: needinfo?(iann_bugzilla)
Target Milestone: seamonkey 2.82 → seamonkey 2.83
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: