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]); >> + } >> } >
