176034 2017-08-28 13:45:46 -0700 REGRESSION(r220278): Web Inspector: ContextMenu items are not getting triggered 2017-08-28 17:29:16 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck commit-queue hi inspector-bugzilla-changes joepeck sam webkit-bug-importer oldest_to_newest 1342925 0 joepeck 2017-08-28 13:45:46 -0700 REGRESSION: Web Inspector: Checkbox ContextMenu items are not getting triggered Steps to Reproduce: 1. Inspect this page 2. Show Console 3. Evaluate something in the console so it is non-empty 4. Right click empty space and "Clear Log" => Doesn't work Steps to Reproduce: 1. Inspect this page 2. Show Network Tab 3. Right click the header and try to toggle any column => Doesn't work Notes: • InspectorFrontendAPI's contextMenuItemSelected is not getting called. 1342930 1 joepeck 2017-08-28 13:54:00 -0700 This appears to have regressed with: r220278 https://trac.webkit.org/changeset/220278/webkit 1342935 2 joepeck 2017-08-28 14:03:41 -0700 - if (!ScriptGlobalObject::get(state, "InspectorFrontendAPI", frontendApiObject)) { + auto value = state.lexicalGlobalObject()->get(&state, JSC::Identifier::fromString(&state.vm(), "InspectorFrontendHost")); InspectorFrontendAPI turned into InspectorFrontendHost! 1342973 3 joepeck 2017-08-28 16:09:30 -0700 I spent a bunch of time seeing what it would take to write a test for this. In general it seems we lack tests for selecting context menu items. There are some LayoutTests to show a ContextMenu that use `eventSender.contextClick`. There are no LayoutTests to select an item in the ContextMenu, which seems okay given there might be platform differences there. In this case the Web Inspector page, not the inspected test page, is what is triggering context menus. It doesn't have `eventSender`, `internals`, etc. We can show a ContextMenu with `InspectorFrontendHost.showContextMenu`, but selecting an item is not immediately possible. My best idea would be to introduce `InspectorFrontendHost.testContextMenuSelectItemAtIndex` but I don't like that. It adds surface area to an object we want try to keep as small as possible since its APIs extend the power of the Inspector frontend page. 1342974 4 319215 joepeck 2017-08-28 16:10:33 -0700 Created attachment 319215 [PATCH] Proposed Fix 1342979 5 319215 hi 2017-08-28 16:20:32 -0700 Comment on attachment 319215 [PATCH] Proposed Fix r=me 1342996 6 319215 commit-queue 2017-08-28 17:27:58 -0700 Comment on attachment 319215 [PATCH] Proposed Fix Clearing flags on attachment: 319215 Committed r221283: <http://trac.webkit.org/changeset/221283> 1342997 7 commit-queue 2017-08-28 17:28:00 -0700 All reviewed patches have been landed. Closing bug. 1343000 8 webkit-bug-importer 2017-08-28 17:29:16 -0700 <rdar://problem/34122845> 319215 2017-08-28 16:10:33 -0700 2017-08-28 17:27:58 -0700 [PATCH] Proposed Fix context-menu-1.patch text/plain 1562 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAxOWFhNzgwZDg4Yy4uMTc4YzMxOGFlNDkgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNCBAQAorMjAxNy0wOC0yOCAgSm9zZXBoIFBlY29yYXJvICA8cGVjb3Jhcm9AYXBwbGUu Y29tPgorCisgICAgICAgIFJFR1JFU1NJT04ocjIyMDI3OCk6IFdlYiBJbnNwZWN0b3I6IENvbnRl eHRNZW51IGl0ZW1zIGFyZSBub3QgZ2V0dGluZyB0cmlnZ2VyZWQKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE3NjAzNAorCisgICAgICAgIFJldmlld2Vk IGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogaW5zcGVjdG9yL0luc3BlY3RvckZyb250 ZW5kSG9zdC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpJbnNwZWN0b3JGcm9udGVuZEhvc3Q6OnNo b3dDb250ZXh0TWVudSk6CisgICAgICAgIFJlc3BvbnNlcyBnbyB0aHJvdWdoIEluc3BlY3RvckZy b250ZW5kQVBJIG5vdCBJbnNwZWN0b3JGcm9udGVuZEhvc3QuCisKIDIwMTctMDgtMjggIENhcmxv cyBBbGJlcnRvIExvcGV6IFBlcmV6ICA8Y2xvcGV6QGlnYWxpYS5jb20+CiAKICAgICAgICAgW0dU S10gW1dQRV0gUmVuYW1lIEV2ZW50SGFuZGxlckdsaWIgdG8gRXZlbnRIYW5kbGVyR0xpYgpkaWZm IC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL0luc3BlY3RvckZyb250ZW5kSG9zdC5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvSW5zcGVjdG9yRnJvbnRlbmRIb3N0LmNwcApp bmRleCA0MjcxOWEzNWZiZi4uZDQ5ODc3YTk2MTkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3Jl L2luc3BlY3Rvci9JbnNwZWN0b3JGcm9udGVuZEhvc3QuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3Jl L2luc3BlY3Rvci9JbnNwZWN0b3JGcm9udGVuZEhvc3QuY3BwCkBAIC0zODQsNyArMzg0LDcgQEAg dm9pZCBJbnNwZWN0b3JGcm9udGVuZEhvc3Q6OnNob3dDb250ZXh0TWVudShFdmVudCYgZXZlbnQs IFZlY3RvcjxDb250ZXh0TWVudUl0ZW0KICAgICBBU1NFUlQobV9mcm9udGVuZFBhZ2UpOwogCiAg ICAgYXV0byYgc3RhdGUgPSAqZXhlY1N0YXRlRnJvbVBhZ2UoZGVidWdnZXJXb3JsZCgpLCBtX2Zy b250ZW5kUGFnZSk7Ci0gICAgYXV0byB2YWx1ZSA9IHN0YXRlLmxleGljYWxHbG9iYWxPYmplY3Qo KS0+Z2V0KCZzdGF0ZSwgSlNDOjpJZGVudGlmaWVyOjpmcm9tU3RyaW5nKCZzdGF0ZS52bSgpLCAi SW5zcGVjdG9yRnJvbnRlbmRIb3N0IikpOworICAgIGF1dG8gdmFsdWUgPSBzdGF0ZS5sZXhpY2Fs R2xvYmFsT2JqZWN0KCktPmdldCgmc3RhdGUsIEpTQzo6SWRlbnRpZmllcjo6ZnJvbVN0cmluZygm c3RhdGUudm0oKSwgIkluc3BlY3RvckZyb250ZW5kQVBJIikpOwogICAgIEFTU0VSVCh2YWx1ZSk7 CiAgICAgQVNTRVJUKHZhbHVlLmlzT2JqZWN0KCkpOwogICAgIGF1dG8qIGZyb250ZW5kQVBJT2Jq ZWN0ID0gYXNPYmplY3QodmFsdWUpOwo=