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