Closed Bug 233513 Opened 16 years ago Closed 5 years ago

Text zoom in composer

Categories

(SeaMonkey :: Composer, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.31

People

(Reporter: glazou, Assigned: neil)

Details

(Keywords: access)

Attachments

(3 files, 2 obsolete files)

Allow Text Zoom in Composer for accessibility reasons.
Original request by Debbie Pomerance <dpomeran at cpcug dot org>
Status: NEW → ASSIGNED
Priority: -- → P1
Comment on attachment 140954 [details] [diff] [review]
fix by Steve Swanson (steve.swanson@mackichan.com)

peterv, could you sr please ?
Attachment #140954 - Flags: superreview?(peterv)
Comment on attachment 140954 [details] [diff] [review]
fix by Steve Swanson (steve.swanson@mackichan.com)

Can you combine these two lines onto one line:
+    var zoomstr;
+    zoomstr = gPrefs.getCharPref("editor.zoom_factor");
Does this affect MailNews Composer bug 177510 ?
(In reply to comment #5)
>Does this affect MailNews Composer bug 177510 ?
I'm not 100% sure, but I don't think any of these files are used by mail.

Of course, the fix could be ported to message compose...
Comment on attachment 140954 [details] [diff] [review]
fix by Steve Swanson (steve.swanson@mackichan.com)

>Index: editor.js
>===================================================================

> function EditorShutdown()
> {
>+  SetUnicharPref("editor.zoom_factor",getMarkupDocumentViewer().textZoom);

Will this add a pref even when the user doesn't change the zoom factor?

>+function setZoom()
>+{
>+  var zoomfactor = 1.0;
>+  try {
>+    var zoomstr;
>+    zoomstr = gPrefs.getCharPref("editor.zoom_factor");
>+    zoomfactor = parseFloat(zoomstr);
>+  }
>+  catch(ex) {
>+    dump("\nfailed to get zoom_factor pref!\n");

Do we really want to dump when the pref is not set? It's not an error, is it?
How about initializing zoomfactor to 1.0 in the catch clause?

>+  }
>+  getMarkupDocumentViewer().textZoom = zoomfactor;
>+}
>+
You could solve both bugs by adding a default pref in composer.js
pref("editor.zoom_factor", "1");
Comment on attachment 141041 [details] [diff] [review]
in answer to peterv's comments, includes missing pref initialization, carries forward r=daniel@glazman.org

>Index: editor.js
>===================================================================

>+function setZoom()
>+{
>+  var zoomfactor = 1.0;
>+  try {
>+    var zoomstr;
>+    zoomstr = gPrefs.getCharPref("editor.zoom_factor");
>+    zoomfactor = parseFloat(zoomstr);
>+  }
>+  catch(ex) {
>+    dump("\nfailed to get zoom_factor pref!\n");
>+  }
>+  getMarkupDocumentViewer().textZoom = zoomfactor;
>+}

With this rewritten as

function setZoom()
{
  var zoomfactor;
  try {
    var zoomstr = gPrefs.getCharPref("editor.zoom_factor");
    zoomfactor = parseFloat(zoomstr);
  }
  catch(ex) {
    dump("\nfailed to get zoom_factor pref!\n");
    zoomfactor = 1.0;
  }
  getMarkupDocumentViewer().textZoom = zoomfactor;
}

and the getCharPref/SetUnicharPref inconsistency cleared up, sr=peterv. Marking
r=glazman too.
Attachment #141041 - Flags: superreview+
Attachment #141041 - Flags: review+
Attachment #140954 - Flags: superreview?(peterv)
Since this is a one-off thing, and the zoom defaults to 1.0, why not this:
function setZoom()
{
  try {
    var zoomstr = gPrefs.getCharPref("editor.zoom_factor");
    getMarkupDocumentViewer().textZoom = parseFloat(zoomstr);
  }
  catch(ex) {
    dump("\nfailed to get zoom_factor pref!\n");
  }
}
Product: Browser → Seamonkey
update and checkin?
if patch is near you might nominate for SM 1.1 bug 315312
Daniel,
Are you still working on this ?
Rebased against trunk and addressed Comment 11
Attachment #642536 - Flags: review?(neil)
Comment on attachment 642536 [details] [diff] [review]
Rebased against current trunk

Sadly bug 386363 bitrotted this (nontrivially, too...)
Attachment #642536 - Flags: review?(neil)
Assignee: daniel → nobody
Status: ASSIGNED → NEW
Attached patch Possible patch (obsolete) — Splinter Review
I wrote this to test the attachment to bug 1038635, so you'll need to apply that patch or test using a build that predates the regression.

I patched editingOverlay so that the plaintext editor works automatically. (Ensure that you're using the in-tree plaintext editor and haven't accidentally installed an old version as a profile extension.)

The viewZoomOverlay.js change fixes a "TypeError: browser is null" bug where viewZoomOverlay is erroneously assuming a tabbrowser exists.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #8458022 - Flags: review?(iann_bugzilla)
Comment on attachment 8458022 [details] [diff] [review]
Possible patch

When using the zoom keys (Ctrl+0, Ctrl++ and Ctrl+-) in a composer window (other than the HTML source tab), this also affects the browser window. Would this expected behaviour?
Flags: needinfo?(neil)
(In reply to Ian Neal from comment #18)
> When using the zoom keys (Ctrl+0, Ctrl++ and Ctrl+-) in a composer window
> (other than the HTML source tab), this also affects the browser window.
> Would this expected behaviour?

Sorry, I don't understand the question, can you provide detailed STR?
Flags: needinfo?(neil)
(In reply to neil@parkwaycc.co.uk from comment #19)
> (In reply to Ian Neal from comment #18)
> > When using the zoom keys (Ctrl+0, Ctrl++ and Ctrl+-) in a composer window
> > (other than the HTML source tab), this also affects the browser window.
> > Would this expected behaviour?
> 
> Sorry, I don't understand the question, can you provide detailed STR?

1/ Open a web page, say "http://www.seamonkey-project.org/start/".
2/ File, Edit Page.
3/ Make sure that you can see the web page behind the composer page.
4/ Use Ctrl++ and Ctrl+- to Zoom in and out.
5/ Note what happens to the web page and composer page

Expected Result
1/ Only composer page zooms in and out.

Actual Result
1/ Both web page and composer page zoom in and out (except when you have the HTML source tab selected).
Flags: needinfo?(neil)
Aha, so what you're seeing here is content preferences - if you change the zoom in any document from www.seamonkey-project.org in any tab or browser window then they all change. I'd better work out how to disable that on a per-window basis.
Flags: needinfo?(neil)
Attached patch Revised patchSplinter Review
OK, so what I had to do was create a dummy FullZoom object for editor which only has the public methods and forwards them directly to the ZoomManager, except for setOther which I had to split off into a separate function.

While I was there I removed a function which wasn't used and doesn't work. (SeaMonkey exposes the preference in the Preferences window instead.)
Attachment #8458022 - Attachment is obsolete: true
Attachment #8458022 - Flags: review?(iann_bugzilla)
Attachment #8461206 - Flags: review?(iann_bugzilla)
Attachment #8461206 - Flags: review?(iann_bugzilla) → review+
a=me for CLOSED TREE
Pushed comm-central changeset 35cf87730d43.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.31
You need to log in before you can comment on or make changes to this bug.