79769 - Application input field on helper app dialog should be disabled on Mac

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect, P3)

Description

[Bill Law]

The spec for the new helper app dialog specifies that the application name input
field should be disabled on the Macintosh.  It is not, thereby
enabling/tempting/tricking/? Mac users into entering path names rather than
pressing the "Choose..." button.

We need to determine that Mozilla is running on a Mac and disable this field.

Comment 1

[Bill Law]

Note that this flaw is also exhibited by the "New Type" and "Edit Type" helper
app prefs dialogs.

Comment 2

[Bill Law]

*** Bug 79847 has been marked as a duplicate of this bug. ***

Comment 3

[Paul Chen]

marking p3 and mozilla1.0

Comment 4

[Paul Chen]

moving to mozilla0.9.8

Comment 5

[Paul Chen]

pushing out to mozilla1.0.1

Comment 6

[Asa Dotzler [:asa]]

->default owner

Comment 7

[Bill Law]

->target milestone it had

Comment 8

[Boris Zbarsky [:bzbarsky]]

This should be fixed (at least for some of the dialogs involved) now that bug
86640 landed.  Could someone please test?

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 188949 has been marked as a duplicate of this bug. ***

Comment 10

[Philip K. Warren]

Attachment: Fix

Bug #86640 didn't fix this completely. We need to disable it explicitly in the
toggleChoice function which is called when radio elements are clicked. This
should fix this problem.

Comment 11

[Aaron Leventhal]

Comment on attachment 111433 [details] [diff] [review]
Fix

>     toggleChoice : function () {
>         // See what option is selected.
>         this.dialogElement( "chooseApp" ).disabled = 
>             this.dialogElement( "appPath" ).disabled = !this.dialogElement( "openUsing" ).selected;
>+
>+        // If we're running on the Mac, disable the application <textbox>.
>+        if ( this.mDialog.navigator.platform.indexOf( "Mac" ) != -1 ) {
>+            this.dialogElement( "appPath" ).disabled = true;
>+        }

this.dialogElement("appPath").disabled is getting set twice, so it could get
set from true and then false unnecessarily.
That's making unnecessary work for reflow. Don't know if it would flash from
non-disabled to disabled state each time.

Anyway, it would be cleaner to just set that at most 1 time for each call of
toggleChoice.
Fix that, and r=aaronl

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 111433 [details] [diff] [review]
Fix

sr=bzbarsky if aaronl's issue is fixed.

Comment 13

[Philip K. Warren]

Taking ownership of bug.

Comment 14

[Philip K. Warren]

Attachment: Updated patch

Only sets the disabled value once for each element. This patch has been updated
to work with changes made in Bug 188931 (uses the mIsMac variable).

Comment 15

[Aaron Leventhal]

Comment on attachment 111563 [details] [diff] [review]
Updated patch

if ( !this.mIsMac ) {
+		 this.dialogElement( "appPath" ).disabled = false;
How about this.dialogElement"appPath" ).disabled = this.mIsMac;

Comment 16

[Aaron Leventhal]

+        if ( this.dialogElement( "openUsing" ).selected ) {
+            
+            // Enable chooseApp button
+            this.dialogElement( "chooseApp" ).disabled = false;
+            
+            // Enable the application <textbox> if we are not on a Mac
+            if ( !this.mIsMac ) {
+                this.dialogElement( "appPath" ).disabled = false;
+            }
+        }
+        else {
+            this.dialogElement( "chooseApp" ).disabled = true;
+            this.dialogElement( "appPath" ).disabled = true;
+        }
var openUsing = this.dialogElement( "openUsing" ).selected;
this.dialogElement( "chooseApp" ).disabled = !openUsing;
this.dialogElement( "appPath" ).disabled = !openUsing || this.isMac;

Comment 17

[Philip K. Warren]

Attachment: Use aaron's suggestion

Comment 18

[Aaron Leventhal]

Comment on attachment 111567 [details] [diff] [review]
Use aaron's suggestion

r=aaronl

Comment 19

[Philip K. Warren]

Fixed.

Comment 20

[Chris Petersen]

Verified in the 2003-01-15-08 trunk build under 10.2.3.