Bug 1345081 - update use of spellchecker 'editable' flags, r=zombie
☠☠ backed out by 831d2c0e7bf9 ☠ ☠
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Fri, 12 Jan 2018 18:51:41 +0000
changeset 400979 5421387a997d4ed4dfb3238aa59f34dd371dd305
parent 400978 60e4c1c5216d827199806579ad88f0223063c54c
child 400980 1edfa7376a6fa28282be22fa84e7d9ca3cd7d76e
push id99272
push userapavel@mozilla.com
push dateFri, 26 Jan 2018 17:52:07 +0000
treeherdermozilla-inbound@1ee8edcdd50d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszombie
bugs1345081
milestone60.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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/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/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)}`);
+    }
+  },
 };