Input field bundles no longer work in popup editor #2299

Closed
dmoagx opened this Issue Oct 20, 2015 · 14 comments

Projects

None yet

2 participants

@dmoagx
Collaborator
dmoagx commented Oct 20, 2015

From Twitter:

Input field bundles used to work in 1.0.2 in the popup field editor, too, but don't anymore in 1.1.

@dmoagx dmoagx added the Bug label Oct 20, 2015
@dmoagx
Collaborator
dmoagx commented Oct 20, 2015

Ok, 50% of that issue was me removing the "mainWindow" state from the field editor and the other 50% goes to whoever assumed the mainWindow would always be the keyWindow.

@dmoagx dmoagx added a commit that referenced this issue Oct 20, 2015
@dmoagx dmoagx Fix bundle commands no longer working in popup field editor (and poss…
…ibly other cases) (#2299)

Replacing some [NSApp mainWindow] with their actually intended calls [NSApp keyWindow] and [view window]
ffbf7a4
@dmoagx dmoagx added a commit that referenced this issue Oct 21, 2015
@dmoagx dmoagx Fix bundle commands no longer working in popup field editor (and poss…
…ibly other cases) (#2299)

Replacing some [NSApp mainWindow] with their actually intended calls [NSApp keyWindow] and [view window]
b089a0d
@dmoagx
Collaborator
dmoagx commented Oct 21, 2015

Hopefully that won't cause any side effects again…

@dmoagx dmoagx added this to the 1.1.1 milestone Oct 30, 2015
@markcarver

@dmoagx this is a pretty serious bug. The ability to deserialize large PHP arrays/objects is now impossible inside the stable released versions of the app (which is what I use the app for 90% of the time).

I just tried the nightly build and I can confirm the above commits have sort of fixed this issue. It will work if you use the main or context menu to manually select the bundle, however the key bindings still do not work after the pop-up sheet shows up.

This is the main menu for the bundle. Notice there is no key binding, which may be part of the issue?
screen shot 2015-11-26 at 6 08 54 am

This is the context menu inside the pop-up sheet (right click), which shows the key binding, but doesn't work:
screen shot 2015-11-26 at 6 08 43 am

Also, is it possible that this issue (once fully fixed) be released before the 1.1.1 milestone or in place of it?

@markcarver

I'm also receiving the following crash on some larger PHP arrays. I've redacted the full contents of the array, but it appears that it's attempting to assign a system variable (to be used on the shell script)? Possible it's not fully escaping the value properly?


NSInvalidArgumentException

*** -[NSFileManager fileSystemRepresentationWithPath:]: conversion failed for SP_CURRENT_WORD=a:64:{...}

(
    0   CoreFoundation                      0x00007fff885e1e32 __exceptionPreprocess + 178
    1   libobjc.A.dylib                     0x00007fff87af44fa objc_exception_throw + 48
    2   CoreFoundation                      0x00007fff8864865d +[NSException raise:format:] + 205
    3   Foundation                          0x00007fff9665604d -[NSFileManager fileSystemRepresentationWithPath:] + 332
    4   Foundation                          0x00007fff967127f3 -[NSConcreteTask launchWithDictionary:] + 1332
    5   Sequel Pro                          0x000000010014c0d4 +[SPBundleCommandRunner runBashCommand:withEnvironment:atCurrentDirectoryPath:callerInstance:contextInfo:error:] + 3508
    6   Sequel Pro                          0x000000010008b5fd -[NSTextView(SPTextViewAdditions) executeBundleItemForInputField:] + 3973
    7   libsystem_trace.dylib               0x00007fff94bf1082 _os_activity_initiate + 75
    8   AppKit                              0x00007fff98b12811 -[NSApplication sendAction:to:from:] + 460
    9   AppKit                              0x00007fff98b125ab -[NSMenuItem _corePerformAction] + 336
    10  AppKit                              0x00007fff98b1230b -[NSCarbonMenuImpl performActionWithHighlightingForItemAtIndex:] + 114
    11  libsystem_trace.dylib               0x00007fff94bf1082 _os_activity_initiate + 75
    12  AppKit                              0x00007fff98ba4b74 -[NSMenu performActionForItemAtIndex:] + 131
    13  AppKit                              0x00007fff98ba4ae7 -[NSMenu _internalPerformActionForItemAtIndex:] + 35
    14  AppKit                              0x00007fff98ba493f -[NSCarbonMenuImpl _carbonCommandProcessEvent:handlerCallRef:] + 107
    15  AppKit                              0x00007fff98a49099 NSSLMMenuEventHandler + 708
    16  HIToolbox                           0x00007fff96b1e98e _ZL23DispatchEventToHandlersP14EventTargetRecP14OpaqueEventRefP14HandlerCallRec + 1231
    17  HIToolbox                           0x00007fff96b1de18 _ZL30SendEventToEventTargetInternalP14OpaqueEventRefP20OpaqueEventTargetRefP14HandlerCallRec + 404
    18  HIToolbox                           0x00007fff96b33df6 SendEventToEventTarget + 40
    19  HIToolbox                           0x00007fff96b7dd2e _ZL18SendHICommandEventjPK9HICommandjjhPKvP20OpaqueEventTargetRefS5_PP14OpaqueEventRef + 411
    20  HIToolbox                           0x00007fff96ba90f7 SendMenuCommandWithContextAndModifiers + 59
    21  HIToolbox                           0x00007fff96ba90a8 SendMenuItemSelectedEvent + 188
    22  HIToolbox                           0x00007fff96ba8f84 _ZL19FinishMenuSelectionP13SelectionDataP10MenuResultS2_ + 96
    23  HIToolbox                           0x00007fff96b87f11 _ZL19PopUpMenuSelectCoreP8MenuData5PointdS1_tjPK4RecttjS4_S4_PK14__CFDictionaryPK10__CFStringPP13OpaqueMenuRefPt + 1794
    24  HIToolbox                           0x00007fff96b86fec _ZL26_HandlePopUpMenuSelection8P13OpaqueMenuRefP14OpaqueEventRefj5PointtjPK4RecttS6_S6_PK14__CFDictionaryPK10__CFStringPS0_Pt + 610
    25  HIToolbox                           0x00007fff96b86be3 _HandlePopUpMenuSelectionWithDictionary + 287
    26  AppKit                              0x00007fff98b9b98c _NSSLMPopUpCarbonMenu3 + 6168
    27  AppKit                              0x00007fff98cbd4c1 -[NSCarbonMenuImpl _popUpContextMenu:withEvent:forView:withFont:] + 200
    28  AppKit                              0x00007fff98cbd326 -[NSMenu _popUpContextMenu:withEvent:forView:withFont:] + 194
    29  AppKit                              0x00007fff9905e09c -[NSView _showMenuForEvent:] + 85
    30  AppKit                              0x00007fff98cb779e -[NSView rightMouseDown:] + 100
    31  AppKit                              0x00007fff98cb772c -[NSTextView rightMouseDown:] + 181
    32  AppKit                              0x00007fff99070b79 -[NSWindow _reallySendEvent:isDelayedEvent:] + 2108
    33  AppKit                              0x00007fff98ab5b8d -[NSWindow sendEvent:] + 517
    34  Sequel Pro                          0x000000010013907f -[SPWindow sendEvent:] + 842
    35  AppKit                              0x00007fff98a35bb6 -[NSApplication sendEvent:] + 2683
    36  AppKit                              0x00007fff9889cd9a -[NSApplication run] + 796
    37  AppKit                              0x00007fff98865fbe NSApplicationMain + 1176
    38  Sequel Pro                          0x00000001000019c4 start + 52
    39  ???                                 0x0000000000000002 0x0 + 2
)
@dmoagx
Collaborator
dmoagx commented Nov 27, 2015
SP_CURRENT_WORD=a:64:{...}

How many characters did it have?

@markcarver

85,811

@dmoagx
Collaborator
dmoagx commented Nov 27, 2015

OK, the length shouldn't be a problem, but your string most likely contains special characters.
Can you check for NUL and possibly others?

@markcarver

Yes, I see a lot of NUL characters. The DB is managed by the Drupal CMS, so not much control over that. Is there a way to filter those out before sending it through?

@dmoagx
Collaborator
dmoagx commented Nov 28, 2015

Let's continue the discussion on the exception in #2342.

The issue with the keyboard shortcut most likely has something to with the fact that "Shift+cmd+D" is already assigned to "Go to Database…".

@markcarver

Ok... so I just tried re-assigning my bundle to something like Cmd+D, but apparently it thinks that's what "Go to Database..." is actually assigned to instead:
monosnap 2015-11-28 23-02-17

@dmoagx
Collaborator
dmoagx commented Nov 29, 2015

Huh, that's interesting.

I just looked at the code and it looks like OS X interprets:

  • cmd+D => cmd+shift+D
  • cmd+shift+d => cmd+shift+D
  • cmd+d => cmd+D

(the different combinations are only accessible via code) while our version of ShortcutRecorder assumes d and D are the same thing.
Well, another reason for #2300

@markcarver

Hm, odd indeed. I suppose this issue could get closed then (as the above commits actually fixed the problem). I'm still a bit confused as to which shortcut "Go to Database..." should be using though.

If it's any consolation, the bundle I use has always been assigned to cmd+shift+D. I really don't remember what "Go to Database..." was before (as I rarely used that), but I would imagine it should only be cmd+d, yes?

@dmoagx
Collaborator
dmoagx commented Nov 29, 2015

The database selection always has been cmd+shift+D, but there was no main menu item for this before, which is probably why you are only seeing it now.

You can try this workaround:
Go to System Prefs > Keyboard > Shortcuts > App Shortcuts and remap Go to Database… to some other key (note that the … is a single ellipsis character and not three dots).
Then in Sequel Pro assign cmd+D to your Bundle.
Afterwards remove the App shortcut from System Prefs again.

That should work fine. The bug only affects assigning keyboard shortcuts.

@dmoagx
Collaborator
dmoagx commented Feb 20, 2016
@dmoagx dmoagx closed this Feb 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment