After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 452316 - [testing required] should have a "fullscreen" checkbox
[testing required] should have a "fullscreen" checkbox
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: magnification
unspecified
Other All
: Normal enhancement
: 2.22.0
Assigned To: Rich Burridge
Orca Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-06-29 17:17 UTC by Samuel Thibault
Modified: 2008-07-22 19:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Revision #1. (38.30 KB, patch)
2007-12-03 19:06 UTC, Rich Burridge
none Details | Review
alternate/demonstration version (39.22 KB, patch)
2007-12-04 23:01 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review
Revision #3 (59.76 KB, patch)
2007-12-05 22:07 UTC, Rich Burridge
none Details | Review
Revision #4 (61.23 KB, patch)
2007-12-05 22:18 UTC, Rich Burridge
none Details | Review
Revision #5 (59.68 KB, patch)
2007-12-05 22:56 UTC, Rich Burridge
none Details | Review
Revision #6 (62.44 KB, patch)
2007-12-06 03:32 UTC, Rich Burridge
none Details | Review
Revision #7 (92.63 KB, patch)
2007-12-06 12:48 UTC, Rich Burridge
accepted-commit_now Details | Review
Revision #8 (91.21 KB, patch)
2007-12-06 15:56 UTC, Rich Burridge
committed Details | Review

Description Samuel Thibault 2007-06-29 17:17:12 UTC
Starting from gnome-mag 0.14.1 and the composite extension, fullscreen magnification is possible without trickering with X server configuration.

When the composite extension is available and gnome-mag is recent enough, orca should hence have a "fullscreen" checkbox that just sets the destination window to the whole screen (left/right/top/bottom controls should hence get desactivated in such case).
Comment 1 Willie Walker 2007-07-08 15:47:31 UTC
Aside from providing the convenience of automatically setting the left/right/top/bottom controls and destination window, are there any special gnome-mag calls Orca needs to make if full screen is enabled?
Comment 2 Samuel Thibault 2007-07-08 16:02:58 UTC
I don't know, but from gnome-mag's magnifier source code, there doesn't seem to be.
Comment 3 Joanmarie Diggs (IRC: joanie) 2007-08-06 14:46:51 UTC
On a fully-updated gusty box with the latest gnome-mag from svn trunk, setting the zoomer controls to be the dimensions of the screen seemed to be all that I needed to do. 
Comment 4 Rich Burridge 2007-12-03 19:06:06 UTC
Created attachment 100134 [details] [review]
Revision #1.

Here's a first cut at this fix.

I have some questions/concerns:

1/ I've added the "Full Screen Magnification" checkbox in
   as the first item on the Magnifier Preferences Pane,
   just after the "Enable Magnifier" checkbox. 

   Q. Is this where we want it? If not, then where? 
   Q. Is the working correct? If not, what should it be?

2/ If the user goes from full screen to non-full screen
   magnification, I'm currently resetting the left zoomer
   position to be screenWidth / 2. 

   Q. Is this what we want? If no, then what should happen here?

3/ I'm unable to test this on a multi-screen setup.

   Q. Could somebody try this for me please?
Comment 5 Joanmarie Diggs (IRC: joanie) 2007-12-03 19:34:05 UTC
>    Q. Is this where we want it? If not, then where? 

Not being a UI person, I couldn't tell you what the exact location should be.  However I personally think it should be in reasonably close proximity to the Zoomer spin buttons for a couple of reasons:

1. Those two creatures are very closely related in function.

2. At higher magnification, a user is unlikely to notice that
   toggling the checkbox state toggles the availability of the
   zoomer spin buttons.

>    Q. Is the working correct? If not, what should it be?

Seems to be working correctly on my laptop.  Although I haven't tested with composite disabled.  I'll try that next.

>    Q. Is this what we want? If no, then what should happen here?

Again, this is just me, but I think it would be cool if enabling full screen magnification had no impact on the values of the spin buttons.  Maybe I want to turn full screen magnification on for one task and then switch back to my regular zoomer that occupies the bottom left corner of my monitor.  Rather than changing the values, could we check the availability of the spin buttons and if they're not available go to full screen mode?
 
>    Q. Could somebody try this for me please?

I will look at it shortly. 
Comment 6 Rich Burridge 2007-12-03 19:46:00 UTC
> Rather than changing the values, could we check the availability of the 
> spin buttons and if they're not available go to full screen mode?

How do we do that? I've (so far) just implemented your suggested fix in
comment #3:

  "On a fully-updated gusty box with the latest gnome-mag from svn trunk, 
   setting the zoomer controls to be the dimensions of the screen seemed 
   to be all that I needed to do." 
Comment 7 Joanmarie Diggs (IRC: joanie) 2007-12-03 20:22:44 UTC
> How do we do that? I've (so far) just implemented your suggested fix in
> comment #3:
> 
>   "On a fully-updated gusty box with the latest gnome-mag from svn trunk, 
>    setting the zoomer controls to be the dimensions of the screen seemed 
>    to be all that I needed to do." 

I'm sorry.  :-(  That's not a suggested fix.  That's a confirmation of Samuel's observation in comment #2 in response to the question Will asked in comment #1 (i.e. It seems safe to conclude that there are no special calls we need to make in Orca because merely setting the zoomer controls as described caused us to be in "fullscreen mode").

As for how do we do that?  I dunno.  I hadn't given it any more thought other than that it might be worth doing. ;-)  I can look later if you'd like.  In the meantime....

Just did some tests with composite disabled on my laptop.  I'm wondering if our fullscreen checkbox should be greyed out if we're not "full screen capable".  gnome-mag now exposes that information to us (see bug #481009), though it looks as though you may need to get gnome-mag from trunk to try it for yourself.

Onto the multi-head tests....
Comment 8 Joanmarie Diggs (IRC: joanie) 2007-12-03 21:18:41 UTC
w.r.t. the multi-head stuff:

1. You're good with Xinerama.

2. You're good without Xinerama for the most part....

I haven't actually read your patch yet, but it seems as though you're not grabbing the target display dimensions.

                    Tri-head Xinerama
        __________   ________________   __________
       |          | |                | |          | 
       |          | |                | |          | 
       |          | |                | |          | 
       |__________| |________________| |__________|

        <--1280-->      <--1680-->      <--1280-->
Xorg:    Screen 1        Screen 0        Screen 2

Configuration:
  Full screen magnification unchecked
  Source display: :0.1 or :0.2 (doesn't matter which)
  Target display: :0.0
  Top, Left, Bottom, Right: 0, 0, 1050, 1680

Checking the Full screen magnification results in: 0, 0, 1024, 1280

If I reverse source and target, I get the same results.  BUT when I then uncheck the checkbox, Left goes to 840 (halving the source rather than the target).  Right stays at 1280.

HTH.
Comment 9 Joanmarie Diggs (IRC: joanie) 2007-12-03 21:27:44 UTC
Just to eliminate (pre-empt) any confusion, I copied and pasted the above diagram from a different bug of mine.  That should not say Xinerama.  Xinerama was disabled in the aforementioned tests.  Sorry!
Comment 10 Rich Burridge 2007-12-03 22:23:03 UTC
> I haven't actually read your patch yet, but it seems as though you're not
> grabbing the target display dimensions.

Correct. I just used what's currently done in settings.py
when setting up those four initial zoomer positions. We
certainly can take into consideration the target display's
dimensions.

Thanks for your feedback.

I'll now wait for Mike and Will to give their feedback too,
before I proceed. Especially you Mike, as our UI guy.
Comment 11 Joanmarie Diggs (IRC: joanie) 2007-12-04 23:01:49 UTC
Created attachment 100209 [details] [review]
alternate/demonstration version

Rich, this is your original patch with the following minor modifications:

1. In orca_gui_prefs.py, just handle the fullscreen setting and leave
   the zoomers as they are.

2. In mag.py ask for the source display bounds ever-so-slightly earlier
   and use them to figure out our screen real estate.  Then, based on
   the fullscreen setting and whether or not we are fullscreen capable,
   assign zoomer dimensions.

Still need to try this on my multi-head system, but it seems to work on my laptop with composite enabled as well as with it disabled.

Thoughts?
Comment 12 Joanmarie Diggs (IRC: joanie) 2007-12-04 23:29:55 UTC
It passes the multi-head test both with Xinerama enabled and disabled.

And while I haven't tried it yet, I believe this approach will make it quite simple to create an unbound command to switch quickly between fullscreen mode and partial screen mode as part of RFE/bug #501414, should we choose to implement that.
Comment 13 Rich Burridge 2007-12-04 23:33:13 UTC
Thanks Joanie. I'll look at it tomorrow (and create a slightly adjusted
version of your patch to move the "full screen" checkbox nearer the
zoomer positions).
Comment 14 Willie Walker 2007-12-05 16:29:06 UTC
From the Orca user's list (http://mail.gnome.org/archives/orca-list/2007-November/msg00262.html), we have a request to make it easier to set the magnifier zoomer size/location:

"The other way would be to change the zoomer setting options to a more user-friendly set of options.  Like zoomer position = top half, bottom half, left half or right half."

I think this sounds like a decent idea that might need to be done as part of this bug.  I'm guessing this could be a combobox with behavior something like the following:

Zoomer position: [Full Screen, Top Half, Bottom Half, Left Half, Right Half, Custom...]

Where the [.....] thing is a combo box and the items are the available items.  If one selects anything except "Custom...", the Zoomer position settings in the dialog will be grayed, but will be populated to reflect the values associated with the item selected.  If "Custom..." is selected, the position settings will be ungrayed, allowing the user to select them.  In addition, if Full Screen support is not possible, then this item should not be available for selection.
Comment 15 Rich Burridge 2007-12-05 22:07:24 UTC
Created attachment 100355 [details] [review]
Revision #3

This hopefully implements everything required to fix this bug and
add in the new enhancement.
Comment 16 Rich Burridge 2007-12-05 22:18:22 UTC
Created attachment 100356 [details] [review]
Revision #4

Pylint'ed version (testing out Will's new pylint.sh script).
Comment 17 Joanmarie Diggs (IRC: joanie) 2007-12-05 22:41:41 UTC
Still testing, but one minor nit:  At team meeting we decided that full screen mode would be the default.  When I try this, right half seems to be default.
Comment 18 Rich Burridge 2007-12-05 22:56:41 UTC
Created attachment 100361 [details] [review]
Revision #5

Make default full-screen.
Comment 19 Joanmarie Diggs (IRC: joanie) 2007-12-05 23:39:11 UTC
Default full-screen fix confirmed.

Three more "nits" but then I think I'm done: :-)

1. You can edit the text displayed in the combo box.

2. 'i' is underlined making Alt+i look like the accelerator.  But Alt+i isn't working for me.

3. (Super minor) On our other combo boxes, the label is aligned with the center of the combo box; on the Zoomer Position combo box, the label is top-aligned.

The functionality is really cool though!
Comment 20 Rich Burridge 2007-12-06 00:00:48 UTC
Thanks. 1 and 2 I'll look at tomorrow, but for 3, the
label is top-aligned to align with the "Zoomer Settings"
label (where that label is is part of the frame surronding 
those widgets. It's also bold for the same reason. If this 
isn't what we want, then it's easy to change.
Comment 21 Willie Walker 2007-12-06 01:33:09 UTC
This is looking really good, and I see the same things Joanie does.  In any case, when those things get resolved, we'll have some nice enhancements that are the result of direct user feedback.  I like that.  :-)
Comment 22 Rich Burridge 2007-12-06 02:24:30 UTC
> This is looking really good, and I see the same things Joanie does.

What would you like done about #3? center-aligned vs top-aligned?
normal vs bold for the label?
Comment 23 Rich Burridge 2007-12-06 03:32:38 UTC
Created attachment 100366 [details] [review]
Revision #6

Here's a version that hopefully fixes those three problems.

Let me know if there is still something that needs changing.
Comment 24 Joanmarie Diggs (IRC: joanie) 2007-12-06 04:02:58 UTC
All three fixes confirmed.

And I do think the combo box looks much better now that the label is not bold and is centered w.r.t. the combo box.  Thanks!

You know what I'm wondering though.... Given that the zoomer position (including the spin buttons) are zoomer settings, perhaps:

1. Rename "zoomer position" to merely "position"

2. Nudge the position label and combo box down a bit so that they are vertically aligned with Scale Factor.  Also nudge it a bit to the left so that it's closer to the Scale Factor combo box.

3. The four zoomer spin buttons could then be moved en masse to the right of the zoomer location.

Or not.... :-) Merely tossing it out there for consideration.
Comment 25 Rich Burridge 2007-12-06 12:48:03 UTC
Created attachment 100379 [details] [review]
Revision #7

Implement suggestions from comment #24.
Comment 26 Willie Walker 2007-12-06 13:11:26 UTC
(In reply to comment #25)
> Created an attachment (id=100379) [edit]
> Revision #7
> 
> Implement suggestions from comment #24.
> 

I think this looks much better, modulo the run_pylint.sh mods that were included by accident. Thanks!
Comment 27 Joanmarie Diggs (IRC: joanie) 2007-12-06 14:32:15 UTC
> I think this looks much better

Agreed.  Thanks Rich!!
Comment 28 Rich Burridge 2007-12-06 15:56:48 UTC
Created attachment 100395 [details] [review]
Revision #8

Version without the pylint.sh changes.
Comment 29 Rich Burridge 2007-12-06 15:57:24 UTC
Thanks for the feedback. Closing as FIXED.