From af1fcf56340afc2d48fde3824ba0ef1abe01fd1a Mon Sep 17 00:00:00 2001
From: Neil Deakin <neil@mozilla.com>
Date: Thu, 4 Mar 2021 08:50:30 +0000
Subject: [PATCH] Bug 1686931, merge view image and view background image
 commands on context menu, r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D103775
---
 browser/base/content/browser-context.inc      |   5 -
 browser/base/content/nsContextMenu.js         |  42 +++---
 .../test/contextMenu/browser_contextmenu.js   | 132 ++++++++++++++----
 .../contextMenu/browser_contextmenu_input.js  |   4 -
 .../test/contextMenu/subtst_contextmenu.html  |   5 +-
 .../content/test/general/browser_bug734076.js |   6 +-
 .../locales/en-US/browser/browserContext.ftl  |   4 -
 ...browser_test_view_image_data_navigation.js |   6 +-
 8 files changed, 140 insertions(+), 64 deletions(-)

diff --git a/browser/base/content/browser-context.inc b/browser/base/content/browser-context.inc
index 22ce1d6c7e5c..6798aa803fae 100644
--- a/browser/base/content/browser-context.inc
+++ b/browser/base/content/browser-context.inc
@@ -236,11 +236,6 @@
         <menupopup id="context-sendpagetodevice-popup"
                    onpopupshowing="(() => { gSync.populateSendTabToDevicesMenu(event.target, gBrowser.currentURI.spec, gBrowser.contentTitle); })()"/>
       </menu>
-      <menuseparator id="context-sep-viewbgimage"/>
-      <menuitem id="context-viewbgimage"
-                data-l10n-id="main-context-menu-view-background-image"
-                oncommand="gContextMenu.viewBGImage(event);"
-                onclick="checkForMiddleClick(this, event);"/>
       <menu id="fill-login"
             label="&fillLoginMenu.label;"
             label-login="&fillLoginMenu.label;"
diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js
index b5d3f028baf9..19e01d236b07 100644
--- a/browser/base/content/nsContextMenu.js
+++ b/browser/base/content/nsContextMenu.js
@@ -185,10 +185,9 @@ class nsContextMenu {
 
     // Assign what's _possibly_ needed from `context` sent by ContextMenuChild.jsm
     // Keep this consistent with the similar code in ContextMenu's _setContext
-    this.bgImageURL = context.bgImageURL;
     this.imageDescURL = context.imageDescURL;
     this.imageInfo = context.imageInfo;
-    this.mediaURL = context.mediaURL;
+    this.mediaURL = context.mediaURL || context.bgImageURL;
     this.webExtBrowserType = context.webExtBrowserType;
 
     this.canSpellCheck = context.canSpellCheck;
@@ -532,11 +531,23 @@ class nsContextMenu {
     );
 
     // View image depends on having an image that's not standalone
-    // (or is in a frame), or a canvas.
-    this.showItem(
-      "context-viewimage",
-      (this.onImage && (!this.inSyntheticDoc || this.inFrame)) || this.onCanvas
-    );
+    // (or is in a frame), or a canvas. If this isn't an image, check
+    // if there is a background image.
+    let showViewImage =
+      (this.onImage && (!this.inSyntheticDoc || this.inFrame)) || this.onCanvas;
+    let showBGImage =
+      this.hasBGImage &&
+      !this.hasMultipleBGImages &&
+      !this.inSyntheticDoc &&
+      !this.inPDFViewer &&
+      !this.isContentSelected &&
+      !this.onImage &&
+      !this.onCanvas &&
+      !this.onVideo &&
+      !this.onAudio &&
+      !this.onLink &&
+      !this.onTextInput;
+    this.showItem("context-viewimage", showViewImage || showBGImage);
 
     // Save image depends on having loaded its content.
     this.showItem("context-saveimage", this.onLoadedImage || this.onCanvas);
@@ -547,11 +558,13 @@ class nsContextMenu {
     this.showItem("context-copyimage-contents", this.onImage);
 
     // Copy image location depends on whether we're on an image.
-    this.showItem("context-copyimage", this.onImage);
+    this.showItem("context-copyimage", this.onImage || showBGImage);
 
     // Send media URL (but not for canvas, since it's a big data: URL)
-    this.showItem("context-sendimage", this.onImage);
+    this.showItem("context-sendimage", this.onImage || showBGImage);
 
+    // Open the link to more details about the image. Does not apply to
+    // background images.
     this.showItem(
       "context-viewimagedesc",
       this.onImage && this.imageDescURL !== ""
@@ -637,17 +650,6 @@ class nsContextMenu {
       this.onVideo && (!this.inSyntheticDoc || this.inFrame)
     );
     this.setItemAttr("context-viewvideo", "disabled", !this.mediaURL);
-
-    // View background image depends on whether there is one, but don't make
-    // background images of a stand-alone media document available.
-    this.showItem(
-      "context-viewbgimage",
-      shouldShow &&
-        !this.hasMultipleBGImages &&
-        !this.inSyntheticDoc &&
-        !this.inPDFViewer
-    );
-    document.getElementById("context-viewbgimage").disabled = !this.hasBGImage;
   }
 
   initMiscItems() {
diff --git a/browser/base/content/test/contextMenu/browser_contextmenu.js b/browser/base/content/test/contextMenu/browser_contextmenu.js
index 185f390ca02c..78971801e6ba 100644
--- a/browser/base/content/test/contextMenu/browser_contextmenu.js
+++ b/browser/base/content/test/contextMenu/browser_contextmenu.js
@@ -167,10 +167,6 @@ add_task(async function test_plaintext() {
     null,
     "---",
     null,
-    "context-viewbgimage",
-    false,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -680,10 +676,6 @@ add_task(async function test_iframe() {
     null,
     "---",
     null,
-    "context-viewbgimage",
-    false,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -1304,10 +1296,6 @@ add_task(async function test_pagemenu() {
       null,
       "---",
       null,
-      "context-viewbgimage",
-      false,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1374,10 +1362,6 @@ add_task(async function test_dom_full_screen() {
       null,
       "---",
       null,
-      "context-viewbgimage",
-      false,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1463,10 +1447,6 @@ add_task(async function test_pagemenu2() {
       null,
       "---",
       null,
-      "context-viewbgimage",
-      false,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1776,10 +1756,6 @@ add_task(async function test_srcdoc() {
     null,
     "---",
     null,
-    "context-viewbgimage",
-    false,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -2038,6 +2014,114 @@ add_task(async function test_svg_relative_link() {
   ]);
 });
 
+add_task(async function test_background_image() {
+  await test_contextmenu("#test-background-image", [
+    "context-navigation",
+    null,
+    [
+      "context-back",
+      false,
+      "context-forward",
+      false,
+      "context-reload",
+      true,
+      "context-bookmarkpage",
+      true,
+    ],
+    null,
+    "---",
+    null,
+    "context-viewimage",
+    true,
+    "context-copyimage",
+    true,
+    "context-sendimage",
+    true,
+    "---",
+    null,
+    "context-savepage",
+    true,
+    ...(hasPocket ? ["context-pocket", true] : []),
+    "---",
+    null,
+    "context-sendpagetodevice",
+    true,
+    [],
+    null,
+    "---",
+    null,
+    "context-selectall",
+    true,
+    "---",
+    null,
+    "context-viewsource",
+    true,
+  ]);
+
+  // Don't show image related context menu commands for links with background images.
+  await test_contextmenu("#test-background-image-link", [
+    "context-openlinkintab",
+    true,
+    ...(hasContainers ? ["context-openlinkinusercontext-menu", true] : []),
+    // We need a blank entry here because the containers submenu is
+    // dynamically generated with no ids.
+    ...(hasContainers ? ["", null] : []),
+    "context-openlink",
+    true,
+    "context-openlinkprivate",
+    true,
+    "---",
+    null,
+    "context-bookmarklink",
+    true,
+    "context-savelink",
+    true,
+    "context-copylink",
+    true,
+    "---",
+    null,
+    "context-searchselect",
+    true,
+    "context-searchselect-private",
+    true,
+    "---",
+    null,
+    "context-sendlinktodevice",
+    false,
+    [],
+    null,
+  ]);
+
+  // Don't show image related context menu commands when there is a selection
+  // with background images.
+  await test_contextmenu(
+    "#test-background-image",
+    [
+      "context-copy",
+      true,
+      "context-selectall",
+      true,
+      "---",
+      null,
+      "context-searchselect",
+      true,
+      "context-searchselect-private",
+      true,
+      "---",
+      null,
+      "context-print-selection",
+      true,
+      "context-viewpartialsource-selection",
+      true,
+    ],
+    {
+      async preCheckContextMenuFn() {
+        await selectText("#test-background-image");
+      },
+    }
+  );
+});
+
 add_task(async function test_cleanup_html() {
   gBrowser.removeCurrentTab();
 });
diff --git a/browser/base/content/test/contextMenu/browser_contextmenu_input.js b/browser/base/content/test/contextMenu/browser_contextmenu_input.js
index 5a0d9fe84a66..39500fcabef3 100644
--- a/browser/base/content/test/contextMenu/browser_contextmenu_input.js
+++ b/browser/base/content/test/contextMenu/browser_contextmenu_input.js
@@ -225,10 +225,6 @@ add_task(
           null,
           "---",
           null,
-          "context-viewbgimage",
-          false,
-          "---",
-          null,
           "context-selectall",
           null,
           "---",
diff --git a/browser/base/content/test/contextMenu/subtst_contextmenu.html b/browser/base/content/test/contextMenu/subtst_contextmenu.html
index 68f2bf4faee6..9d3483d48b63 100644
--- a/browser/base/content/test/contextMenu/subtst_contextmenu.html
+++ b/browser/base/content/test/contextMenu/subtst_contextmenu.html
@@ -86,5 +86,8 @@ if ("ShadowRoot" in this) {
 <svg id="svg-with-relative-link" width=10 height=10><a xlink:href="/"><circle cx="50%" cy="50%" r="50%" fill="blue"/></a></svg>
 <svg id="svg-with-relative-link2" width=10 height=10><a xlink:href="/" xlink:type="simple"><circle cx="50%" cy="50%" r="50%" fill="green"/></a></svg>
 <svg id="svg-with-relative-link3" width=10 height=10><a href="/"><circle cx="50%" cy="50%" r="50%" fill="red"/></a></svg>
-</body>
+<span id="test-background-image" style="background-image: url('ctxmenu-image.png')">Text with background
+  <a id='test-background-image-link' href="about:blank">image</a>
+  .
+</span></body>
 </html>
diff --git a/browser/base/content/test/general/browser_bug734076.js b/browser/base/content/test/general/browser_bug734076.js
index 184aebab45db..8c00f2785417 100644
--- a/browser/base/content/test/general/browser_bug734076.js
+++ b/browser/base/content/test/general/browser_bug734076.js
@@ -19,7 +19,7 @@ add_task(async function() {
 
   let tests = [
     {
-      name: "view background image",
+      name: "view image with background image",
       url: "http://mochi.test:8888/",
       element: "body",
       go() {
@@ -31,7 +31,7 @@ add_task(async function() {
             contentBody.style.backgroundImage =
               "url('" + arg.writeDomainURL + "')";
 
-            return "context-viewbgimage";
+            return "context-viewimage";
           }
         );
       },
@@ -41,7 +41,7 @@ add_task(async function() {
         ) {
           Assert.ok(
             !content.document.body.textContent,
-            "no domain was inherited for view background image"
+            "no domain was inherited for view image with background image"
           );
         });
       },
diff --git a/browser/locales/en-US/browser/browserContext.ftl b/browser/locales/en-US/browser/browserContext.ftl
index caa2ca3aeacc..a644e6360861 100644
--- a/browser/locales/en-US/browser/browserContext.ftl
+++ b/browser/locales/en-US/browser/browserContext.ftl
@@ -298,10 +298,6 @@ main-context-menu-send-to-device =
     .label = Send Page to Device
     .accesskey = n
 
-main-context-menu-view-background-image =
-    .label = View Background Image
-    .accesskey = w
-
 main-context-menu-generate-new-password =
     .label = Use Generated Password…
     .accesskey = G
diff --git a/dom/security/test/general/browser_test_view_image_data_navigation.js b/dom/security/test/general/browser_test_view_image_data_navigation.js
index 19548b63aa0c..c75b773a5f73 100644
--- a/dom/security/test/general/browser_test_view_image_data_navigation.js
+++ b/dom/security/test/general/browser_test_view_image_data_navigation.js
@@ -50,10 +50,10 @@ add_task(async function test_right_click_open_bg_image() {
       true
     );
 
-    // simulate right-click->view-bg-image
+    // simulate right-click->view-image
     BrowserTestUtils.waitForEvent(document, "popupshown", false, event => {
       // These are operations that must be executed synchronously with the event.
-      document.getElementById("context-viewbgimage").doCommand();
+      document.getElementById("context-viewimage").doCommand();
       event.target.hidePopup();
       return true;
     });
@@ -67,7 +67,7 @@ add_task(async function test_right_click_open_bg_image() {
     let spec = gBrowser.selectedBrowser.currentURI.spec;
     ok(
       spec.startsWith("data:image/svg+xml;"),
-      "data:image/svg navigation allowed through right-click view-bg-image"
+      "data:image/svg navigation allowed through right-click view-image with background image"
     );
   });
 });