+1 :) Thanks, Niall
On 27/03/2009 04:55, Jedy Wang wrote: > I consider this as one +1:-) Do I still need anther one? > > Regards, > > Jedy > On Thu, 2009-03-26 at 06:18 -0400, Willie Walker wrote: > >> Works awesome using the latest webrev at >> http://cr.opensolaris.org/~jedy/7436/. >> >> Many thanks Jedy! >> >> Will >> >> Jedy Wang wrote: >> >>> On Wed, 2009-03-25 at 10:11 +0000, Willie Walker wrote: >>> >>>> Thanks Jedy! I had difficulty applying the last patch you attached, but >>>> did some hand editing of the file. The complete diff I have from hg >>>> trunk is attached. >>>> >>>> With this patch in place, what I now see happening is that when the >>>> Welcome screen appears, focus is now transferred to the invisible disk >>>> radio button on the next page. Very strange, but the net result is that >>>> Orca now speaks the disk information when the user is still on the >>>> "Welcome" screen. >>>> >>> Hi Will, >>> >>> I take a look at the code. The installer uses a timeout callback >>> (partition_discovery_monitor) to discover the disks and focus on the >>> default disk button. In some cases the call back will return before the >>> disk screen is shown. I updated the webrev and make sure that the disk >>> button grabs the foucs only after the disk screen is shown. >>> >>> Regards, >>> >>> Jedy >>> >>>> I looked at the rest of the gui-install code and I'm having trouble >>>> figuring out why this is happening. From what I can tell, the code path >>>> for setting focus should only be called when the disk screen appears. >>>> But, I only gave it a 5 minute look. :-( Do you have any ideas? >>>> >>>> Will >>>> >>>> Jedy Wang wrote: >>>> >>>>> Hi Will, >>>>> >>>>> I just updated my patch and the webrev. Now the fix should not have such >>>>> problem. A diff between the old patch and the now one is also attached. >>>>> >>>>> Regards, >>>>> >>>>> Jedy >>>>> On Tue, 2009-03-24 at 12:03 +0000, Willie Walker wrote: >>>>> >>>>>> Hi Jedy: >>>>>> >>>>>> This definitely makes the radio button behavior work much better and is >>>>>> consistent with how GTK+ radio button groups work. Many thanks! >>>>>> >>>>>> One thing I noticed, however, is that when going from the initial >>>>>> "Welcome" screen to the disk selection screen, the focus now remains on >>>>>> the "Next" button rather than going to a disk. :-( >>>>>> >>>>>> Many thanks again for your work here, >>>>>> >>>>>> Will >>>>>> >>>>>> Jedy Wang wrote: >>>>>> >>>>>>> Hi all, >>>>>>> >>>>>>> Please review the patch which fixes bug 7436 - disk radio button >>>>>>> behaves like toggle button and breaks Orca. >>>>>>> >>>>>>> The fix makes disk buttons behave like radio button by toggling them >>>>>>> when they get focus. The patch also fix the default widget problem for >>>>>>> disk screen. For details, please refer to >>>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=7436 >>>>>>> >>>>>>> webrev: >>>>>>> http://cr.opensolaris.org/~jedy/7436/ >>>>>>> >>>>>>> Regards, >>>>>>> >>>>>>> Jedy >>>>>>> >>>>>>> >>>>>>> >>>> plain text document attachment (installation-disk-screen.diff) >>>> diff -r 7473788ad5aa usr/src/cmd/gui-install/src/installation-disk-screen.c >>>> --- a/usr/src/cmd/gui-install/src/installation-disk-screen.c Tue Mar >>>> 24 18:20:22 2009 -0600 >>>> +++ b/usr/src/cmd/gui-install/src/installation-disk-screen.c Wed Mar >>>> 25 09:50:40 2009 +0000 >>>> @@ -534,6 +534,17 @@ >>>> disk_selection_set_active_disk(disknum); >>>> } >>>> >>>> +/* Toggle the button when it gets the focus. */ >>>> +static gboolean >>>> +installationdisk_diskbutton_focused(GtkWidget *widget, >>>> + GdkEventFocus *event, >>>> + gpointer user_data) >>>> +{ >>>> + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(widget), >>>> + !gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(widget))); >>>> + return (FALSE); >>>> +} >>>> + >>>> /* UI initialisation functoins */ >>>> >>>> void >>>> @@ -1511,6 +1522,10 @@ >>>> G_CALLBACK(installationdisk_diskbutton_toggled), >>>> GINT_TO_POINTER(disknum)); >>>> g_signal_connect(G_OBJECT(diskbuttons[disknum]), >>>> + "focus", >>>> + G_CALLBACK(installationdisk_diskbutton_focused), >>>> + GINT_TO_POINTER(disknum)); >>>> + g_signal_connect(G_OBJECT(diskbuttons[disknum]), >>>> "focus-in-event", >>>> G_CALLBACK(disk_partitioning_button_focus_handler), >>>> GINT_TO_POINTER(disknum)); >>>> @@ -1541,6 +1556,44 @@ >>>> gtk_container_add(GTK_CONTAINER(viewport), hbuttonbox); >>>> } >>>> >>>> +/* >>>> + * Return the index of the default disk >>>> + * or -1 indicating an error. >>>> + * The default disk should be the 1st >>>> + * bootable disk, or the 1st usable disk, >>>> + * or the 1st available disk. >>>> + */ >>>> +static gint >>>> +get_default_disk_index(void) >>>> +{ >>>> + gint chosendisk = -1; >>>> + gint i = 0; >>>> + >>>> + for (i = 0; i< numdisks; i++) { >>>> + if (get_disk_status(i) == DISK_STATUS_OK || >>>> + get_disk_status(i) == DISK_STATUS_CANT_PRESERVE) { >>>> + if (orchestrator_om_disk_is_bootdevice(alldiskinfo[i])) >>>> { >>>> + /* >>>> + * If boot device is found >>>> + * and it's usable, look no further. >>>> + */ >>>> + chosendisk = i; >>>> + break; >>>> + } else if (chosendisk< 0) >>>> + /* >>>> + * fall back to the 1st >>>> + * usable disk >>>> + */ >>>> + chosendisk = i; >>>> + } >>>> + } >>>> + /* fall back to the 1st avaiable disk */ >>>> + if (numdisks> 0&& chosendisk< 0) >>>> + chosendisk = 0; >>>> + return chosendisk; >>>> +} >>>> + >>>> + >>>> static gboolean >>>> partition_discovery_monitor(gpointer user_data) >>>> { >>>> @@ -1567,25 +1620,7 @@ >>>> * Auto select the boot disk, or failing that, the first suitable disk >>>> * and toggle the custom partitioning controls. >>>> */ >>>> - for (i = 0; i< numdisks; i++) { >>>> - if (get_disk_status(i) == DISK_STATUS_OK || >>>> - get_disk_status(i) == DISK_STATUS_CANT_PRESERVE) { >>>> - /* If boot device is found and it's usable, look no >>>> further */ >>>> - if (orchestrator_om_disk_is_bootdevice(alldiskinfo[i])) >>>> { >>>> - chosendisk = i; >>>> - break; >>>> - } else if (chosendisk< 0) >>>> - chosendisk = i; >>>> - } >>>> - } >>>> - >>>> - /* >>>> - * If no suitable disk was found, something still has to be >>>> - * selected because we are using radio buttons. So just select >>>> - * the first device. >>>> - */ >>>> - if (numdisks> 0&& chosendisk< 0) >>>> - chosendisk = 0; >>>> + chosendisk = get_default_disk_index(); >>>> if (chosendisk>= 0) { >>>> gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON( >>>> diskbuttons[chosendisk]), >>>> @@ -1601,6 +1636,7 @@ >>>> /* Force a toggle emission */ >>>> gtk_toggle_button_toggled(GTK_TOGGLE_BUTTON( >>>> diskbuttons[chosendisk])); >>>> + gtk_widget_grab_focus(diskbuttons[chosendisk]); >>>> >>>> #if defined(__i386) >>>> /* Show partitioning options on X86 only */ >>>> @@ -1609,9 +1645,6 @@ >>>> "partitioningvbox")); >>>> #endif >>>> } >>>> - if >>>> (GTK_WIDGET_VISIBLE(MainWindow.InstallationDiskWindow.diskselectiontoplevel)) >>>> - if (diskbuttons&& diskbuttons[0]) >>>> - gtk_widget_grab_focus(diskbuttons[0]); >>>> >>>> return (FALSE); >>>> } >>>> @@ -2662,12 +2695,20 @@ >>>> >>>> /* >>>> * Set the default widget with focus. >>>> - * The default widget for installation >>>> - * disk screen is the 1st disk button. >>>> + * When activedisk is not set, the default >>>> + * widget for installation disk screen is >>>> + * the 1st bootable disk button, or the 1st >>>> + * usable disk button, or the 1st available >>>> + * disk button. Otherwise activedisk is the >>>> + * default. >>>> */ >>>> void >>>> installationdisk_screen_set_default_focus(void) >>>> { >>>> - if (diskbuttons&& diskbuttons[0]) >>>> - gtk_widget_grab_focus(diskbuttons[0]); >>>> + activedisk = get_default_disk_index(); >>>> + if (activedisk< 0) >>>> + activedisk = get_default_disk_index(); >>>> + if (activedisk>= 0) { >>>> + gtk_widget_grab_focus(diskbuttons[activedisk]); >>>> + } >>>> } >>>> > > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20090327/827f99ff/attachment.html>
