Bug 1345081 - update use of spellchecker 'editable' flags, r?zombie draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 12 Jan 2018 18:51:41 +0000
changeset 748260 bb593291748c0c4bb35bfbed61d1674e91b8a0b4
parent 748015 232dbd5acc3f2be2be59eddb3e7ef1d00d9111a0
push id97102
push usergijskruitbosch@gmail.com
push dateMon, 29 Jan 2018 12:35:23 +0000
reviewerszombie
bugs1345081
milestone60.0a1
Bug 1345081 - update use of spellchecker 'editable' flags, r?zombie MozReview-Commit-ID: Hxgc0UuIOPj
browser/base/content/nsContextMenu.js
browser/components/extensions/ext-menus.js
browser/components/extensions/test/browser/browser_ext_contextMenus.js
browser/components/extensions/test/browser/browser_ext_menus_events.js
browser/components/extensions/test/browser/context.html
browser/modules/ContextMenu.jsm
toolkit/modules/InlineSpellChecker.jsm
toolkit/modules/tests/browser/browser_InlineSpellChecker.js
--- a/browser/base/content/nsContextMenu.js
+++ b/browser/base/content/nsContextMenu.js
@@ -111,17 +111,18 @@ nsContextMenu.prototype = {
         inFrame: this.inFrame,
         isTextSelected: this.isTextSelected,
         onTextInput: this.onTextInput,
         onLink: this.onLink,
         onImage: this.onImage,
         onVideo: this.onVideo,
         onAudio: this.onAudio,
         onCanvas: this.onCanvas,
-        onEditableArea: this.onEditableArea,
+        onEditable: this.onEditable,
+        onSpellcheckable: this.onSpellcheckable,
         onPassword: this.onPassword,
         srcUrl: this.mediaURL,
         frameUrl: gContextMenuContentData ? gContextMenuContentData.docLocation : undefined,
         pageUrl: this.browser ? this.browser.currentURI.spec : undefined,
         linkText: this.linkTextStr,
         linkUrl: this.linkURL,
         selectionText: this.isTextSelected ? this.selectionInfo.fullText : undefined,
         frameId: this.frameOuterWindowID,
@@ -191,27 +192,28 @@ nsContextMenu.prototype = {
     this.linkURL             = context.linkURL;
     this.linkURI             = this.getLinkURI(); // can't send; regenerate
 
     this.onAudio             = context.onAudio;
     this.onCanvas            = context.onCanvas;
     this.onCompletedImage    = context.onCompletedImage;
     this.onCTPPlugin         = context.onCTPPlugin;
     this.onDRMMedia          = context.onDRMMedia;
-    this.onEditableArea      = context.onEditableArea;
+    this.onEditable          = context.onEditable;
     this.onImage             = context.onImage;
     this.onKeywordField      = context.onKeywordField;
     this.onLink              = context.onLink;
     this.onLoadedImage       = context.onLoadedImage;
     this.onMailtoLink        = context.onMailtoLink;
     this.onMathML            = context.onMathML;
     this.onMozExtLink        = context.onMozExtLink;
     this.onNumeric           = context.onNumeric;
     this.onPassword          = context.onPassword;
     this.onSaveableLink      = context.onSaveableLink;
+    this.onSpellcheckable    = context.onSpellcheckable;
     this.onTextInput         = context.onTextInput;
     this.onVideo             = context.onVideo;
 
     this.target = this.isRemote ? context.target : document.popupNode;
     this.targetAsCPOW = context.targetAsCPOW;
 
     this.principal = context.principal;
     this.frameOuterWindowID = context.frameOuterWindowID;
@@ -575,17 +577,17 @@ nsContextMenu.prototype = {
     // dictionary list
     this.showItem("spell-dictionaries", showDictionaries);
     if (canSpell) {
       var dictMenu = document.getElementById("spell-dictionaries-menu");
       var dictSep = document.getElementById("spell-language-separator");
       let count = InlineSpellCheckerUI.addDictionaryListToMenu(dictMenu, dictSep);
       this.showItem(dictSep, count > 0);
       this.showItem("spell-add-dictionaries-main", false);
-    } else if (this.onEditableArea) {
+    } else if (this.onSpellcheckable) {
       // when there is no spellchecker but we might be able to spellcheck
       // add the add to dictionaries item. This will ensure that people
       // with no dictionaries will be able to download them
       this.showItem("spell-language-separator", showDictionaries);
       this.showItem("spell-add-dictionaries-main", showDictionaries);
     } else {
       this.showItem("spell-add-dictionaries-main", false);
     }
--- a/browser/components/extensions/ext-menus.js
+++ b/browser/components/extensions/ext-menus.js
@@ -433,17 +433,17 @@ var gMenuBuilder = {
 global.actionContextMenu = function(contextData) {
   contextData.tab = tabTracker.activeTab;
   contextData.pageUrl = contextData.tab.linkedBrowser.currentURI.spec;
   gMenuBuilder.buildActionContextMenu(contextData);
 };
 
 const contextsMap = {
   onAudio: "audio",
-  onEditableArea: "editable",
+  onEditable: "editable",
   inFrame: "frame",
   onImage: "image",
   onLink: "link",
   onPassword: "password",
   isTextSelected: "selection",
   onVideo: "video",
 
   onBookmark: "bookmark",
@@ -483,17 +483,17 @@ function addMenuEventInfo(info, contextD
     info.mediaType = "image";
   }
   if (contextData.frameId !== undefined) {
     info.frameId = contextData.frameId;
   }
   if (contextData.onBookmark) {
     info.bookmarkId = contextData.bookmarkId;
   }
-  info.editable = contextData.onEditableArea || contextData.onPassword || false;
+  info.editable = contextData.onEditable || false;
   if (includeSensitiveData) {
     if (contextData.onLink) {
       info.linkText = contextData.linkText;
       info.linkUrl = contextData.linkUrl;
     }
     if (contextData.onAudio || contextData.onImage || contextData.onVideo) {
       info.srcUrl = contextData.srcUrl;
     }
--- a/browser/components/extensions/test/browser/browser_ext_contextMenus.js
+++ b/browser/components/extensions/test/browser/browser_ext_contextMenus.js
@@ -228,32 +228,95 @@ add_task(async function() {
     editable: true,
   };
 
   result = await extension.awaitMessage("onclick");
   checkClickInfo(result);
   result = await extension.awaitMessage("browser.contextMenus.onClicked");
   checkClickInfo(result);
 
+  extensionMenuRoot = await openExtensionContextMenu("#readonly-text");
+
+  // Check some menu items.
+  items = extensionMenuRoot.getElementsByAttribute("label", "editable");
+  is(items.length, 0, "contextMenu item for text input element was not found (context=editable fails for readonly items)");
+
+  // Hide the popup "manually" because there's nothing to click.
+  await closeContextMenu();
+
+  // Test "editable" context on type=tel and type=number items, and OnClick data property.
+  extensionMenuRoot = await openExtensionContextMenu("#call-me-maybe");
+
+  // Check some menu items.
+  items = extensionMenuRoot.getElementsByAttribute("label", "editable");
+  is(items.length, 1, "contextMenu item for text input element was found (context=editable)");
+  editable = items[0];
+
+  // Click on ext-editable item and check the click results.
+  await closeExtensionContextMenu(editable);
+
+  expectedClickInfo = {
+    menuItemId: "ext-editable",
+    pageUrl: PAGE,
+    editable: true,
+  };
+
+  result = await extension.awaitMessage("onclick");
+  checkClickInfo(result);
+  result = await extension.awaitMessage("browser.contextMenus.onClicked");
+  checkClickInfo(result);
+
+  extensionMenuRoot = await openExtensionContextMenu("#number-input");
+
+  // Check some menu items.
+  items = extensionMenuRoot.getElementsByAttribute("label", "editable");
+  is(items.length, 1, "contextMenu item for text input element was found (context=editable)");
+  editable = items[0];
+
+  // Click on ext-editable item and check the click results.
+  await closeExtensionContextMenu(editable);
+
+  expectedClickInfo = {
+    menuItemId: "ext-editable",
+    pageUrl: PAGE,
+    editable: true,
+  };
+
+  result = await extension.awaitMessage("onclick");
+  checkClickInfo(result);
+  result = await extension.awaitMessage("browser.contextMenus.onClicked");
+  checkClickInfo(result);
+
   extensionMenuRoot = await openExtensionContextMenu("#password");
   items = extensionMenuRoot.getElementsByAttribute("label", "password");
   is(items.length, 1, "contextMenu item for password input element was found (context=password)");
   let password = items[0];
   await closeExtensionContextMenu(password);
   expectedClickInfo = {
     menuItemId: "ext-password",
     pageUrl: PAGE,
     editable: true,
   };
 
   result = await extension.awaitMessage("onclick");
   checkClickInfo(result);
   result = await extension.awaitMessage("browser.contextMenus.onClicked");
   checkClickInfo(result);
 
+  extensionMenuRoot = await openExtensionContextMenu("#noneditablepassword");
+  items = extensionMenuRoot.getElementsByAttribute("label", "password");
+  is(items.length, 1, "contextMenu item for password input element was found (context=password)");
+  password = items[0];
+  await closeExtensionContextMenu(password);
+  expectedClickInfo.editable = false;
+  result = await extension.awaitMessage("onclick");
+  checkClickInfo(result);
+  result = await extension.awaitMessage("browser.contextMenus.onClicked");
+  checkClickInfo(result);
+
   // Select some text
   await ContentTask.spawn(gBrowser.selectedBrowser, { }, async function(arg) {
     let doc = content.document;
     let range = doc.createRange();
     let selection = content.getSelection();
     selection.removeAllRanges();
     let textNode = doc.getElementById("img1").previousSibling;
     range.setStart(textNode, 0);
--- a/browser/components/extensions/test/browser/browser_ext_menus_events.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus_events.js
@@ -367,22 +367,22 @@ add_task(async function test_show_hide_f
 
 add_task(async function test_show_hide_password() {
   await testShowHideEvent({
     menuCreateParams: {
       title: "password item",
       contexts: ["password"],
     },
     expectedShownEvent: {
-      contexts: ["password", "all"],
+      contexts: ["editable", "password", "all"],
       editable: true,
       frameId: 0,
     },
     expectedShownEventWithPermissions: {
-      contexts: ["password", "all"],
+      contexts: ["editable", "password", "all"],
       editable: true,
       frameId: 0,
       pageUrl: PAGE,
     },
     async doOpenMenu() {
       await openContextMenu("#password");
     },
     async doCloseMenu() {
--- a/browser/components/extensions/test/browser/context.html
+++ b/browser/components/extensions/test/browser/context.html
@@ -14,17 +14,21 @@
   <p>
     <a href="image-around-some-link">
       <img src="ctxmenu-image.png" id="img-wrapped-in-link">
     </a>
   </p>
 
   <p>
     <input type="text" id="edit-me"><br>
+    <input id="readonly-text" type="text" readonly >
+    <input id="call-me-maybe" type="tel" value="0123456789">
+    <input id="number-input" type="number" value="123456789"><br>
     <input type="password" id="password">
+    <input id="noneditablepassword" type="password" readonly >
   </p>
   <iframe id="frame" src="context_frame.html"></iframe>
   <p id="longtext">Sed ut perspiciatis unde omnis iste natus error sit
   voluptatem accusantium doloremque laudantium, totam rem aperiam, eaque
   ipsa quae ab illo inventore veritatis et quasi architecto beatae vitae dicta
   sunt explicabo. Nemo enim ipsam voluptatem quia voluptas sit aspernatur aut
   odit aut fugit, sed quia consequuntur magni dolores eos qui ratione voluptatem
   sequi nesciunt. Neque porro quisquam est, qui dolorem ipsum quia dolor sit
--- a/browser/modules/ContextMenu.jsm
+++ b/browser/modules/ContextMenu.jsm
@@ -575,18 +575,17 @@ class ContextMenu {
       this._cleanContext();
     }
 
     let isRemote = Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT;
 
     if (isRemote) {
       editFlags = SpellCheckHelper.isEditable(aEvent.target, this.content);
 
-      if (editFlags &
-          (SpellCheckHelper.EDITABLE | SpellCheckHelper.CONTENTEDITABLE)) {
+      if (editFlags & SpellCheckHelper.SPELLCHECKABLE) {
         spellInfo = InlineSpellCheckerContent.initContextMenu(aEvent, editFlags, this.global);
       }
 
       // Set the event target first as the copy image command needs it to
       // determine what was context-clicked on. Then, update the state of the
       // commands on the context menu.
       this.global.docShell.contentViewer.QueryInterface(Ci.nsIContentViewerEdit)
                           .setCommandNode(aEvent.target);
@@ -744,27 +743,28 @@ class ContextMenu {
     context.linkURL             = "";
     context.linkURI             = null;
 
     context.onAudio             = false;
     context.onCanvas            = false;
     context.onCompletedImage    = false;
     context.onCTPPlugin         = false;
     context.onDRMMedia          = false;
-    context.onEditableArea      = false;
+    context.onEditable          = false;
     context.onImage             = false;
     context.onKeywordField      = false;
     context.onLink              = false;
     context.onLoadedImage       = false;
     context.onMailtoLink        = false;
     context.onMathML            = false;
     context.onMozExtLink        = false;
     context.onNumeric           = false;
     context.onPassword          = false;
     context.onSaveableLink      = false;
+    context.onSpellcheckable    = false;
     context.onTextInput         = false;
     context.onVideo             = false;
 
     // Remember the node and its owner document that was clicked
     // This may be modifed before sending to nsContextMenu
     context.target = node;
 
     context.principal = context.target.ownerDocument.nodePrincipal;
@@ -868,20 +868,23 @@ class ContextMenu {
       }
 
       if (this._isProprietaryDRM()) {
         context.onDRMMedia = true;
       }
     } else if (editFlags & (SpellCheckHelper.INPUT | SpellCheckHelper.TEXTAREA)) {
       context.onTextInput = (editFlags & SpellCheckHelper.TEXTINPUT) !== 0;
       context.onNumeric = (editFlags & SpellCheckHelper.NUMERIC) !== 0;
-      context.onEditableArea = (editFlags & SpellCheckHelper.EDITABLE) !== 0;
+      context.onEditable = (editFlags & SpellCheckHelper.EDITABLE) !== 0;
       context.onPassword = (editFlags & SpellCheckHelper.PASSWORD) !== 0;
+      context.onSpellcheckable = (editFlags & SpellCheckHelper.SPELLCHECKABLE) !== 0;
 
-      if (context.onEditableArea) {
+      // This is guaranteed to be an input or textarea because of the condition above,
+      // so the no-children flag is always correct. We deal with contenteditable elsewhere.
+      if (context.onSpellcheckable) {
         context.shouldInitInlineSpellCheckerUINoChildren = true;
       }
 
       context.onKeywordField = (editFlags & SpellCheckHelper.KEYWORD);
     } else if (context.target instanceof this.content.HTMLHtmlElement) {
       const bodyElt = context.target.ownerDocument.body;
 
       if (bodyElt) {
@@ -1003,28 +1006,29 @@ class ContextMenu {
       context.inFrame = true;
 
       if (context.target.ownerDocument.isSrcdocDocument) {
           context.inSrcdocFrame = true;
       }
     }
 
     // if the document is editable, show context menu like in text inputs
-    if (!context.onEditableArea) {
+    if (!context.onEditable) {
       if (editFlags & SpellCheckHelper.CONTENTEDITABLE) {
-        // If this._onEditableArea is false but editFlags is CONTENTEDITABLE, then
+        // If this.onEditable is false but editFlags is CONTENTEDITABLE, then
         // the document itself must be editable.
         context.onTextInput       = true;
         context.onKeywordField    = false;
         context.onImage           = false;
         context.onLoadedImage     = false;
         context.onCompletedImage  = false;
         context.onMathML          = false;
         context.inFrame           = false;
         context.inSrcdocFrame     = false;
         context.hasBGImage        = false;
         context.isDesignMode      = true;
-        context.onEditableArea    = true;
+        context.onEditable        = true;
+        context.onSpellcheckable  = true;
         context.shouldInitInlineSpellCheckerUIWithChildren = true;
       }
     }
   }
 }
--- a/toolkit/modules/InlineSpellChecker.jsm
+++ b/toolkit/modules/InlineSpellChecker.jsm
@@ -379,17 +379,18 @@ InlineSpellChecker.prototype = {
     if (this.mRemote)
       this.mRemote.ignoreWord();
     else
       this.mInlineSpellChecker.ignoreWord(this.mMisspelling);
   }
 };
 
 var SpellCheckHelper = {
-  // Set when over a non-read-only <textarea> or editable <input>.
+  // Set when over a non-read-only <textarea> or editable <input>
+  // (that allows text entry of some kind, so not e.g. <input type=checkbox>)
   EDITABLE: 0x1,
 
   // Set when over an <input> element of any type.
   INPUT: 0x2,
 
   // Set when over any <textarea>.
   TEXTAREA: 0x4,
 
@@ -404,16 +405,20 @@ var SpellCheckHelper = {
   CONTENTEDITABLE: 0x20,
 
   // Set when over an <input type="number"> or other non-text field.
   NUMERIC: 0x40,
 
   // Set when over an <input type="password"> field.
   PASSWORD: 0x80,
 
+  // Set when spellcheckable. Replaces `EDITABLE`/`CONTENTEDITABLE` combination
+  // specifically for spellcheck.
+  SPELLCHECKABLE: 0x100,
+
   isTargetAKeywordField(aNode, window) {
     if (!(aNode instanceof window.HTMLInputElement))
       return false;
 
     var form = aNode.form;
     if (!form || aNode.type == "password")
       return false;
 
@@ -438,61 +443,63 @@ var SpellCheckHelper = {
     return aElem.ownerGlobal
                 .getComputedStyle(aElem).getPropertyValue(aProp);
   },
 
   isEditable(element, window) {
     var flags = 0;
     if (element instanceof window.HTMLInputElement) {
       flags |= this.INPUT;
-
       if (element.mozIsTextField(false) || element.type == "number") {
         flags |= this.TEXTINPUT;
+        if (!element.readOnly) {
+          flags |= this.EDITABLE;
+        }
 
         if (element.type == "number") {
           flags |= this.NUMERIC;
         }
 
         // Allow spellchecking UI on all text and search inputs.
         if (!element.readOnly &&
             (element.type == "text" || element.type == "search")) {
-          flags |= this.EDITABLE;
+          flags |= this.SPELLCHECKABLE;
         }
         if (this.isTargetAKeywordField(element, window))
           flags |= this.KEYWORD;
         if (element.type == "password") {
           flags |= this.PASSWORD;
         }
       }
     } else if (element instanceof window.HTMLTextAreaElement) {
       flags |= this.TEXTINPUT | this.TEXTAREA;
       if (!element.readOnly) {
-        flags |= this.EDITABLE;
+        flags |= this.SPELLCHECKABLE | this.EDITABLE;
       }
     }
 
-    if (!(flags & this.EDITABLE)) {
+    if (!(flags & this.SPELLCHECKABLE)) {
       var win = element.ownerGlobal;
       if (win) {
-        var isEditable = false;
+        var isSpellcheckable = false;
         try {
           var editingSession = win.QueryInterface(Ci.nsIInterfaceRequestor)
                                   .getInterface(Ci.nsIWebNavigation)
                                   .QueryInterface(Ci.nsIInterfaceRequestor)
                                   .getInterface(Ci.nsIEditingSession);
           if (editingSession.windowIsEditable(win) &&
               this.getComputedStyle(element, "-moz-user-modify") == "read-write") {
-            isEditable = true;
+            isSpellcheckable = true;
           }
         } catch (ex) {
           // If someone built with composer disabled, we can't get an editing session.
         }
 
-        if (isEditable)
-          flags |= this.CONTENTEDITABLE;
+        if (isSpellcheckable)
+          flags |= this.CONTENTEDITABLE | this.SPELLCHECKABLE;
       }
     }
 
     return flags;
   },
 };
 
 function RemoteSpellChecker(aSpellInfo) {
--- a/toolkit/modules/tests/browser/browser_InlineSpellChecker.js
+++ b/toolkit/modules/tests/browser/browser_InlineSpellChecker.js
@@ -1,14 +1,16 @@
 var InlineSpellChecker;
+var SpellCheckHelper;
 
 function test() {
   let tempScope = {};
   Components.utils.import("resource://gre/modules/InlineSpellChecker.jsm", tempScope);
   InlineSpellChecker = tempScope.InlineSpellChecker;
+  SpellCheckHelper = tempScope.SpellCheckHelper;
 
   ok(InlineSpellChecker, "InlineSpellChecker class exists");
   for (var fname in tests) {
     tests[fname]();
   }
 }
 
 var tests = {
@@ -115,9 +117,36 @@ var tests = {
     is(isc.getDictionaryDisplayName("en-x-ignore-this"), "English", "'en-x-ignore-this' should display as 'English'");
     is(isc.getDictionaryDisplayName("en-x-ignore-this-subtag"), "English", "'en-x-ignore-this-subtag' should display as 'English'");
 
     // Check that both extension and privateuse subtags are ignored.
     todo_is(isc.getDictionaryDisplayName("en-Cyrl-t-en-latn-m0-ungegn-2007-x-ignore-this-subtag"), "English / Cyrillic", "'en-Cyrl-t-en-latn-m0-ungegn-2007-x-ignore-this-subtag' should display as 'English / Cyrillic'");
 
     // XXX: Check grandfathered tags.
   },
+
+  testFlagsForInputs() {
+    const HTML_NS = "http://www.w3.org/1999/xhtml";
+    const {
+      INPUT, EDITABLE, TEXTINPUT, NUMERIC, PASSWORD, SPELLCHECKABLE,
+    } = SpellCheckHelper;
+    const kExpectedResults = {
+      "text": INPUT | EDITABLE | TEXTINPUT | SPELLCHECKABLE,
+      "password": INPUT | EDITABLE | TEXTINPUT | PASSWORD,
+      "search": INPUT | EDITABLE | TEXTINPUT | SPELLCHECKABLE,
+      "url": INPUT | EDITABLE | TEXTINPUT,
+      "tel": INPUT | EDITABLE | TEXTINPUT,
+      "email": INPUT | EDITABLE | TEXTINPUT,
+      "number": INPUT | EDITABLE | TEXTINPUT | NUMERIC,
+      "checkbox": INPUT,
+      "radio": INPUT,
+    };
+
+    for (let [type, expectedFlags] of Object.entries(kExpectedResults)) {
+      let input = document.createElementNS(HTML_NS, "input");
+      input.type = type;
+      let actualFlags = SpellCheckHelper.isEditable(input, window);
+      is(actualFlags, expectedFlags,
+         `For input type "${type}" expected flags ${"0x" + expectedFlags.toString(16)}; ` +
+         `got ${"0x" + actualFlags.toString(16)}`);
+    }
+  },
 };