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