The Description text field in the Bookmarks Info panel (in both bookmark view and folder view) has no focus ring when it is focused. Putting this blocking bug 325880 because it's been suggested it might just be a nib change....

This behaves correctly; NSTextViews don't get focus rings. If it really needs to be "fixed", it would have to be changed to a word-wrapping NSTextField, which would require code changes as well as nib changes. That would also remove the scroll bar though, which may not be desirable if people have long descriptions.

Isn't that what, say, Safari's textareas are? Or is that a custom widget? Otherwise, this sounds like a WONTFIX.

You can't make an NSTextView (or more precisely its NSScrollView) display a focus ring without hackery--try a Google search for "NSTextView focus ring".

Something we discussed on IRC is the "make it an NSTextField instead" option. This has the advantages of a) giving it the requested focus ring and b) making the font match the other fields. The disadvantage would of course be losing the scroll-bar. Personally, I can't imagine ever writing more than 3 or 4 lines of description, and if somebody really wanted to write a lot, they could still access all the text via keyboard, resizing the panel, or dragging over the text. Simon, how do you feel about this idea?

Sure

Comment on attachment 228060 [details] New BookmarkInfoPanel.nib r- on something here; with this nib and patch, 1) I can't loop around in the folder side from Description back to Title with FKA off, and 2) with FKA on, the checkboxes are not in the keyboard loop at all. Both of these things worked fine before this nib/patch combo.

Comment on attachment 228061 [details] [diff] [review] Uses NSTextField instead of NSTextView > // beware that [NSText string] can return a pointer to (mutable) internal strorage >- [mBookmarkItem setItemDescription:[NSString stringWithString:[mBookmarkDescField string]]]; >+ [mBookmarkItem setItemDescription:[NSString stringWithString:[mBookmarkDescField stringValue]]]; Since there are no more NSTexts being sent |string| anymore, remove the comment. Also, you are missing a necessary change from |string| to |stringValue|: > else if ((changedField == mBookmarkDescField) || (changedField == mFolderDescField)) > [mBookmarkItem setItemDescription:[NSString stringWithString:[changedField string]]];

Comment on attachment 228897 [details] New BookmarkInfoPanel.nib Two issues here: 1) This nib still hates me 2) Since we have to mess with it for this bug, perhaps we should standardize the position of the checkboxes in the folder view? The left edge of the left one is 33px from the left edge of the text field, and the right edge of the right one is 25px from the right edge of the text field. I'm not really sure what we might want to do (esp. wrt to l10n); I just noticed that they had different measurements. r=me if you can get someone else on 10.3.9 to verify that the tab chain still works in the folder side (forward and backward, FKA on and off), with or without point 2.

Comment on attachment 228896 [details] [diff] [review] Patch Unfortunately, this bitrotted when the dock-menu changes landed. If you unbit-rot it though, r=me (I applied it manually, and it works fine).

Comment on attachment 228897 [details] New BookmarkInfoPanel.nib The nib has issues though. I don't see the key loop stuff, but the text field needs to have the layout changed from scrollable to wrapping (with word-wrap) or tying a lot of text just gives you one really long line and a lot of empty space.

This is the bitrot police.

This addressed smorgan's comments. Smokey, as for the tabbing thing, would you be OK with landing this nib, on the condition that we back it out if other 10.3 users see the same issues you're seeing? I'm having trouble finding someone who can/will build 10.3, and my debug builds are too damn big for me to upload anywhere for somebody to download and test...

That seems reasonable, I guess.

backing out nibs can suck rocks, esp if someone comes along and changes them. can you do a release build and upload that for other 10.3 testers before it gets landed?

> backing out nibs can suck rocks, esp if someone comes along and changes them. > can you do a release build and upload that for other 10.3 testers before it > gets landed? If someone tells me what I'm supposed to test, I'm happy to build and test this.

Torben: apply the patch and add the nib In the bookmark manager, select a bookmark folder and Get Info. 1) Make sure that you can tab and shift-tab all the way around through the textfields 2) Turn on full keyboard access in System Prefs: Keyboard & Mouse: Keyboard Shortcuts, and make sure that the tab chain also includes the two checkboxes (in both tab/shift tab direction) You should check both 1 and 2 on regular bookmark, too, to be sure, but things work fine for me with regular bookmarks....

I just tested this and I see issues with tabbing :-( However, they're not identical to Smokey's comment 8 and there are issues with tabbing also without this patch (slightly less though): Without the patch I can tab Name -> Description -> Name, the Keyword-field is ignored. Shift-tab from Description takes me (correctly) to Keyword, which I can't Shift-tab out of (tab takes me back to Description though). From Name Shift-tab takes me (correctly again) to Description. With FKA on the check-boxes are correctly in between Description and Name (tabbing and Shift-tabbing). The only change with this patch is that I no longer can tab out of Description, Shift-tabbing takes me to Keyword (with the same problem as above). With FKA on I can Shift-tab from Name to the check-boxes (and from there to Description) but still not out of Description. The Get Info-window for normal bookmarks works as expected in all instances. Since the tab-loop is broken on 10.3 anyway, I suggest we take this patch and handle the rest in a new bug.

Yeah, I filed a bug about tab skipping Keyword and shift-tab getting stuck in Keyword before, but everyone said I was on crack. I really don't know what to do about that nib. :(

Comment on attachment 232767 [details] [diff] [review] r=smorgan patch Pinkerton, are you ok with this? The code changes out the scrolling NSTextView with an NSTextField for description fields. The nib increases the minimum size. However, it may have some weirdness in keyboard support (see comment 8 and comment 21) on 10.3. However, it *already* has some weirdness on 10.3. My proposal is basically that of torben in comment 21 - check this in, and file a followup bug for making the nib right on 10.3

Comment on attachment 232767 [details] [diff] [review] r=smorgan patch sr=pink

Checked in on 1.8branch and trunk.