991757 - Collect UITelemetry on context-menu usage.

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[Blake Winton (:bwinton) (:☕️)]

In bug 971776, we're starting to look into improving the context menus in Firefox.  It would be nice to know how people are currently using them, so that we can account for that in our designs.  It would also be nice to know whether the new designs are having the effect we desire.  Both of these require UITelemetry.  :)

Comment 1

[Chad Weiner [:chadw]]

Blake, to help us prioritize, can you list the specific questions you want answered through UI telemetry?

Comment 2

[Blake Winton (:bwinton) (:☕️)]

In order:

How many people are using the context menu?
  Broken out by type of context menu (i.e. "selected text", "image", "page").
How often is each menu item used?
  Broken out by type of context menu (i.e. "selected text", "image", "page").
How often do users see extra (site-added) context menu items?
  How often do they get clicked on vs. the default items?
  Note: We don't want to record what those items are or where they came from, just whether they exist, possibly how many of them, and how often they get clicked!

Comment 3

[:Gijs (he/him)]

(In reply to Blake Winton (:bwinton) from comment #2)
> In order:
> 
> How many people are using the context menu?
>   Broken out by type of context menu (i.e. "selected text", "image", "page").
> How often is each menu item used?
>   Broken out by type of context menu (i.e. "selected text", "image", "page").

Unfortunately the page context menu is really just one massive thing where we selectively hide/show various items. The implementation lives in http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js .

Can you be more specific about how far you want this broken down? The annoying thing is that implementation-wise this is mostly a massive list of booleans, rather than a particular "type" - so it'd probably involve converting from e.g. "onImage && onLink" to type "image-link" or something like that.

That also means that the decision of how far we want this to be specified is a very important one, because we might miss information in analysis later when people click on a combination of stuff, if we don't clarify the possible categories well.

Because there are so many possibilities, I think we should potentially do some low hanging fruit first, so go for:

"image"
"canvas"
"link"
"image-link"
"canvas-link" (maybe should just be image-link?)
"media" (i.e. video/audio elements)
"text-input"
"selected-text"
"social" (this is anything from social sidebars, where we don't display the navigation elements)
"other"

and then potentially expand "other" if necessary.

Does that make sense and/or do you think I've missed anything important?

Comment 4

[Blake Winton (:bwinton) (:☕️)]

That seems fine to me.  Are the social-sidebars using the same menu?  Interesting.  I might even be okay with not tracking those, since they seem like they would have different usage patterns.  (Although, perhaps capturing those usage patterns would be useful, to split them out from the "normal" usage.)

The one thing that I think is missing which we might want to split out is "page", where you right-click on nothing in particular, and get the new context navigation buttons (from http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#150)…  That might be what you meant by "other", but I'm not sure what else would be in "other".

Comment 5

[:Gijs (he/him)]

(In reply to Blake Winton (:bwinton) from comment #4)
> The one thing that I think is missing which we might want to split out is
> "page", where you right-click on nothing in particular, and get the new
> context navigation buttons (from
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/
> nsContextMenu.js#150)…  That might be what you meant by "other", but I'm not
> sure what else would be in "other".

This was exactly my point - right now anything that doesn't fall in any of the categories I specified earlier gets navigation items. I think we should call it "other" rather than "page" because there are probably still more booleans (that aren't in the massive conditional on line 151) that might be worth investigating in future, but I don't know what they are and I think to start with, this should do. We can file followup bugs if necessary.

Would that work for you? :-)

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Ah, okay, yeah that totally works for me then.  Thanks!  :)

Comment 7

[:Gijs (he/him)]

Marco, would it be OK for me to pick this up? Michael Maslaney and I will be talking about the OS X Yosemite work for bug 1040250 on Tuesday, but I need something to keep me busy in the meantime. :-)

Comment 8

[Marco Mucci [:MarcoM]]

Added to Iteration 34.2

Comment 9

[:Gijs (he/him)]

Attachment: add telemetry for the context menu,

I think this will need a privacy review.

Comment 10

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #9)
> Created attachment 8470949 [details] [diff] [review]
> add telemetry for the context menu,
> 
> I think this will need a privacy review.

... but reading the privacy team's info[0] I'm less sure. Stacy, we're basically going to be using the existing telemetry infrastructure ( https://wiki.mozilla.org/Privacy/Reviews/Telemetry ) to log the number of times users open the context menu, and what (built-in, no add-on stuff!) items users click in there, and whether the page they are on provides custom menu items of its own. We would also record if they click such items provided by the page, but not what items or what page they were on when clicking those items, or any other information identifying the user or the pages they use the menu on. There would also be no information about the sequence of clicks/activations (ie no real "clicktrail"), only per-session total counts of clicks. The only thing sent to mozilla would be simple counts on a per-category basis, translating to information such as: "over this browsing session, the user clicked "open link in new tab" when the context menu was opened on a link 5 times".

Do we need a separate privacy review or not?


[0] https://wiki.mozilla.org/Privacy/HowTo/Privacy_Review

Comment 11

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 8470949 [details] [diff] [review]
add telemetry for the context menu,

I like the "close-without-interaction", and the "withcustom"/"withoutcustom" markers.
I wonder if we want to put these in _countableEvents, since we're counting them…  Hopefully Mike will chime in one way or the other.

Other than that, it looks pretty awesome to me!  :)
Thanks!

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8470949 [details] [diff] [review]
add telemetry for the context menu,

Review of attachment 8470949 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Gijs,

This looks good. Just a few questions before I give my review one way or the other.

I would recommend not putting the items that Blake mentioned in countableEvents. countableEvents is placed under the "toolbars" measurement group, and the new additions wouldn't make much sense there.

We could move countableEvents out from under toolbars to make it more generic, but that'd mean someone would have to re-jig the analysis scripts... and we might have to find some way of versioning this JSON blob. Guh. We might have to do that anyway at some point, but for now, unless we really need the data, I wouldn't chase it down.

::: browser/base/content/browser-context.inc
@@ +2,5 @@
>  # This Source Code Form is subject to the terms of the Mozilla Public
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
> +# NB: IF YOU ADD ITEMS TO THIS FILE, PLEASE UPDATE THE WHITELIST IN

LOUD NOISES.

:)

::: browser/base/content/nsContextMenu.js
@@ +1699,5 @@
>      menuItem.accessKey = gNavigatorBundle.getString("contextMenuSearch.accesskey");
> +  },
> +
> +  _getTelemetryClickInfo: function(aXulMenu) {
> +    let removeHandlers = (function() {

Bonus points if you use fancy arrow functions here instead, and skip the .bind. That's just a personal preference.

@@ +1728,5 @@
> +        this._telemetryClickID = "custom-page-item";
> +      } else {
> +        this._telemetryClickID = (e.target.id || "unknown").replace(/^context-/i, "");
> +      }
> +      removeHandlers();

Isn't this redundant, since the popup will hide whether or not a command is fired? If so, aren't we registering context menu interactions twice?

@@ +1763,5 @@
> +    return "other";
> +  },
> +
> +  _checkTelemetryForMenu: function(aXulMenu) {
> +    this._telemetryClickID = null;

Could we not set this stuff in _initMenu instead? I don't see much value in adding another function for it unless it's being repeatedly called.

::: browser/modules/BrowserUITelemetry.jsm
@@ +221,5 @@
>     * and again.
>     *
>     * @param aKeys the Array of keys to chain Objects together with.
>     * @param aEndWith the value to assign to the last key.
>     * @returns a reference to the second last object in the chain -

Let's add some documentation up here for what "root" is.

@@ +617,5 @@
> +    "spell-add-dictionaries-main", "spell-dictionaries",
> +    "spell-dictionaries-menu", "spell-add-dictionaries",
> +    "bidi-text-direction-toggle", "bidi-page-direction-toggle", "inspect",
> +  ]),
> +  _contextMenuInteractions: {},

Nit - let's add some newlines between these properties.

Comment 13

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #12)
> Comment on attachment 8470949 [details] [diff] [review]
> add telemetry for the context menu,
> 
> Review of attachment 8470949 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hey Gijs,
> 
> This looks good. Just a few questions before I give my review one way or the
> other.
> 
> I would recommend not putting the items that Blake mentioned in
> countableEvents. countableEvents is placed under the "toolbars" measurement
> group, and the new additions wouldn't make much sense there.

Right... hence passing an object to stick stuff onto into countEvents; they shouldn't be in _countableEvents now - were you just responding to the suggestion or was that not clear / did I mess it up? :-)

Comment 14

[:Gijs (he/him)]

(In reply to Mike Conley (:mconley) from comment #12)
> @@ +1728,5 @@
> > +        this._telemetryClickID = "custom-page-item";
> > +      } else {
> > +        this._telemetryClickID = (e.target.id || "unknown").replace(/^context-/i, "");
> > +      }
> > +      removeHandlers();
> 
> Isn't this redundant, since the popup will hide whether or not a command is
> fired? If so, aren't we registering context menu interactions twice?

We're not, because removeHandlers kills off the popuphiding handler. But we could probably leave out the call. I did this earlier because of submenus, but now that we're relying on command events, I think this should be fine.

> 
> @@ +1763,5 @@
> > +    return "other";
> > +  },
> > +
> > +  _checkTelemetryForMenu: function(aXulMenu) {
> > +    this._telemetryClickID = null;
> 
> Could we not set this stuff in _initMenu instead? I don't see much value in
> adding another function for it unless it's being repeatedly called.

There's a lot of different functions called from initItems(), purely for reasons of splitting things up / keeping the code understandable, AFAICT. I figured I'd follow that pattern, but I'm not super attached to it.

Comment 15

[Mike Conley (:mconley) (:⚙️)]

(In reply to :Gijs Kruitbosch (intermittently here 14-15 August; then away until 19th) from comment #13)
> (In reply to Mike Conley (:mconley) from comment #12)
> > Comment on attachment 8470949 [details] [diff] [review]
> > add telemetry for the context menu,
> > 
> > Review of attachment 8470949 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Hey Gijs,
> > 
> > This looks good. Just a few questions before I give my review one way or the
> > other.
> > 
> > I would recommend not putting the items that Blake mentioned in
> > countableEvents. countableEvents is placed under the "toolbars" measurement
> > group, and the new additions wouldn't make much sense there.
> 
> Right... hence passing an object to stick stuff onto into countEvents; they
> shouldn't be in _countableEvents now - were you just responding to the
> suggestion or was that not clear / did I mess it up? :-)

Nope, I was just responding to Blake's comment:

> I wonder if we want to put these in _countableEvents, since we're counting them…

Those should go under your context menu branch, and _not_ in _countableEvents. So long as you do that, it makes sense. :)

(In reply to :Gijs Kruitbosch (intermittently here 14-15 August; then away until 19th) from comment #14)
> There's a lot of different functions called from initItems(), purely for
> reasons of splitting things up / keeping the code understandable, AFAICT. I
> figured I'd follow that pattern, but I'm not super attached to it.

Yeah, alright - sold. I'm fine with it.

Comment 16

[:Gijs (he/him)]

Attachment: add telemetry for the context menu,

Good thing I had to look at it again - I forgot to whitelist the 'no interaction' thing. Works now. :-)

Comment 17

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8473160 [details] [diff] [review]
add telemetry for the context menu,

Review of attachment 8473160 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks Gijs.

Comment 18

[:Gijs (he/him)]

I'd like to go ahead and land this; I don't think the privacy implications are different from the ones for our builtin toolbar buttons (which are tracked in a similar way), but I'll leave the needinfo to be sure. Also adding Benjamin (please see comment #9 / comment 10) per the recent m.governance thread about Fx data collection.

Try push for the sheriffs:

remote:   https://tbpl.mozilla.org/?tree=Try&rev=f70f708ca1e6

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Leaks ahoy.

Comment 20

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #18)
> I'd like to go ahead and land this; I don't think the privacy implications
> are different from the ones for our builtin toolbar buttons (which are
> tracked in a similar way), but I'll leave the needinfo to be sure. Also
> adding Benjamin (please see comment #9 / comment 10) per the recent
> m.governance thread about Fx data collection.

Somehow this doesn't seem to have happened... trying harder this time!

Comment 21

[:Gijs (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #19)
> Leaks ahoy.

Hey, look at that, devtools tests manually construct nsContextMenus (which calls initMenus) and don't bother hiding them or destroying them in any way, but call nsContextMenu.inspectNode() directly (instead of click()ing the context menu item itself or whatever). Which leaks the listeners I'm adding, which, because they're scope-bound, leak the world. Graaaaaaand.

New patch incoming. :-\

Comment 22

[Benjamin Smedberg]

This is fine.

Please document these UITelemetry actions in the tree, preferably in a file similar to dataformat.rst. bug 987333 tracks documenting UITelemetry in general, and I'm allergic to data collection without format docs. I'd really love for somebody to take that!

Do you have a planned expiration for this data? I can't imagine that we need to collect it permanently: since AFAIK UITelemetry doesn't have any auto-expiration system, can you file a followup to disable this collection when we're finished reporting on it?

Comment 23

[:Gijs (he/him)]

Attachment: add telemetry for the context menu,

I opted against refactoring all the consumers so that the constructor just adds a popuphiding listener that does everything all the consumers have cargo-culted around (see browser.xul, web-panels.xul, chat-window.xul). I could file a followup, though - it's just that I suspect that that way lies trouble and this seems like the simpler fix for the moment, without trying to fix everything else about the menu... Checking in with msucan about the devtools tests I'm touching (which have a reason for calling the menu like this, which is that the inspectNode call yields a promise for which the test waits...).

Comment 24

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8475340 [details] [diff] [review]
add telemetry for the context menu,

Review of attachment 8475340 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine to me. Thanks for tracking down those leaks.

Comment 25

[:Gijs (he/him)]

Comment on attachment 8475340 [details] [diff] [review]
add telemetry for the context menu,

Redirecting the devtools portion to Brian per discussion on IRC.

Comment 26

[Brian Grinstead [:bgrins]]

Comment on attachment 8475340 [details] [diff] [review]
add telemetry for the context menu,

Review of attachment 8475340 [details] [diff] [review]:
-----------------------------------------------------------------

The devtools test changes are fine, please update the commit message with r=bgrins

Comment 27

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/4c5e12ef22c6

Comment 28

[Benjamin Smedberg]

Still needs the in-tree docs?

Comment 29

[:Gijs (he/him)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #28)
> Still needs the in-tree docs?

Ugh, yes. I'm sorry, I got distracted with the leaks. I will land these tomorrow.

Comment 30

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4c5e12ef22c6

Comment 31

[:Gijs (he/him)]

Attachment: add BrowserUITelemetry documentation, DONTBUILD because docs-only

I would test this, but ./mach build-docs is broken:

gkruitbosch-16516:fx-team gkruitbosch$ ./mach build-docs ../builds/docs/
Error running mach:

    ['build-docs', '../builds/docs/']

    The error occurred in code that was called by the mach command. This is either
    a bug in the called code itself or in the way that mach is calling it.

    You should consider filing a bug for this issue.

    If filing a bug, please include the full output of mach, including this error
    message.

    The details of the failure are as follows:

    BuildReaderError: 
    ==============================
    ERROR PROCESSING MOZBUILD FILE
    ==============================

    The error occurred while processing the following file:

        /Users/gkruitbosch/dev/fx-team/browser/components/build/moz.build

    The error was triggered on line 15 of this file:

        XPCOMBinaryComponent('browsercomps')

    An error was encountered as part of executing the file itself. The error appears to be the fault of the script.

    The error as reported by Python is:

        ["NameError: name 'XPCOMBinaryComponent' is not defined\n"]


    File "/Users/gkruitbosch/dev/fx-team/tools/docs/mach_commands.py", line 49, in build_docs
      for context in reader.walk_topsrcdir():
    File "/Users/gkruitbosch/dev/fx-team/python/mozbuild/mozbuild/frontend/reader.py", line 788, in walk_topsrcdir
      filesystem_absolute=True, read_tiers=True):
    File "/Users/gkruitbosch/dev/fx-team/python/mozbuild/mozbuild/frontend/reader.py", line 833, in read_mozbuild
      sys.exc_info()[2], sandbox_exec_error=se)

Comment 32

[Benjamin Smedberg]

Comment on attachment 8478961 [details] [diff] [review]
add BrowserUITelemetry documentation,  DONTBUILD because docs-only

I can't vouch for the accuracy of the information here, but this is the basic data I am looking for. Thanks.

Comment 33

[Gregory Szorc [:gps]]

Comment 31 implies your tree or Python bytecode is out of date. That should never happen :/

This should be fixed with a clobber or removal of your .pyc files (`hg st -ni | grep .pyc | xargs rm`).

We'll likely continue seeing bugs like this until we have a proper virtualenv in the build system and we stop writing bytecode into the source directory.

Comment 34

[:Gijs (he/him)]

(In reply to Gregory Szorc [:gps] from comment #33)
> Comment 31 implies your tree or Python bytecode is out of date. That should
> never happen :/
> 
> This should be fixed with a clobber or removal of your .pyc files (`hg st
> -ni | grep .pyc | xargs rm`).
> 
> We'll likely continue seeing bugs like this until we have a proper
> virtualenv in the build system and we stop writing bytecode into the source
> directory.

I ran the above, then ran:

./mach build && ./mach build-docs ../builds/docs

and the same thing happened.

Then I re-ran the pyc cleansing and build-docs, and still saw the same error message.

Comment 35

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8478961 [details] [diff] [review]
add BrowserUITelemetry documentation,  DONTBUILD because docs-only

Review of attachment 8478961 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me

Comment 36

[Gregory Szorc [:gps]]

Oh, I didn't realize this was a |mach build-docs| issue. Yeah, |mach build-docs| is currently busted due to build system changes. Don't worry about this too much. Check in docs at will.

Comment 37

[:Gijs (he/him)]

OK!

https://hg.mozilla.org/integration/fx-team/rev/a8944add4cb6

Comment 38

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a8944add4cb6

Comment 39

[Kamil Jozwiak [:kjozwiak]]

Gijs, is there anything QE can do here for verification? Is the collected data being stored in about:telemetry? If so, could you please add some details? Thanks :)

Comment 40

[:Gijs (he/him)]

(In reply to Kamil Jozwiak [:kjozwiak] from comment #39)
> Gijs, is there anything QE can do here for verification? Is the collected
> data being stored in about:telemetry? If so, could you please add some
> details? Thanks :)

Yes, it's under "Simple Measurements", and then under "UI Telemetry". Specifically, you're looking for the "contextmenu" property, which will look something like:

{
   ...,
   "contextmenu":{
      "__DEFAULT__":{
         "other":{
            "withoutcustom":{
               "close-without-interaction":1
            },
            "withcustom":{
               "close-without-interaction":1
            }
         },
         "link":{
            "withoutcustom":{
               "openlinkintab":1
            }
         }
      }
   }
}

(prettyprinted here for your viewing comfort)

The __DEFAULT__ thing is just the telemetry bucket.
The next thing in the hierarchy is the type of content on which the user content-clicked (a link, an image, etc.).
The next thing down from there is whether there was a custom menu item in the context menu when it opened (I find MDN pages an easy place to check, right clicking those has "Edit page" options added to the menu)
Finally the last thing indicates which item was clicked. Note "close-without-interaction" which means no item was clicked, the IDs of items, a special ID for the page item, and a special ID for an item whose ID was unknown to us (e.g. added by an add-on).

I also added more complete documentation under http://mxr.mozilla.org/mozilla-central/source/browser/docs/UITelemetry.rst . It looks like the generated documentation is still broken ( it's not listed at https://ci.mozilla.org/job/mozilla-central-docs/Tree_Documentation/ ) but if you find that thing hard to read, click the 'raw' link at the top, and copy-paste the contents into an online restructured text renderer (use your search engine to find one, it's been too long so I've forgotten which one I used). :-)

Comment 41

[:Gijs (he/him)]

Oh, and, the number assigned at the deepest point in that hierarchy is a count of how often such clicks occurred (so in my example, the context menu was opened on a "blank bit of page" 2 times, once with custom items there and once without, and both times I closed it without clicking anything, and another time I opened it on a link and opened that link in a new tab).

Comment 42

[Catalin Varga [QA][:VarCat]]

Hi I've tested this on:
FF 34
Build Id:20141113192814

and I've noticed some strange behavior for example sometimes "video-hidestats" is logged under "selection" even if the context menu is clearly "media" eg:

"contextmenu": {
		"__DEFAULT__": {
			"media": {
				"withoutcustom": {
					"media-playbackrate-050x": 1,
					"video-showstats": 2,
					"video-hidestats": 1
				}
			},
			"selection": {
				"withoutcustom": {
					"copy": 1,
					"searchselect": 1,
					"video-hidestats": 1,
					"close-without-interaction": 4,
					"video-showstats": 1,
					"inspect": 1
				}
			},
			"other": {
				"withcustom": {
					"custom-page-item": 1
				}
			}
		}
	} 
also I've noticed that some interactions are logged as "close-without-interaction" even if it's not the case.

Comment 43

[:Gijs (he/him)]

(In reply to Catalin Varga [QA][:VarCat] from comment #42)
> Hi I've tested this on:
> FF 34
> Build Id:20141113192814
> 
> and I've noticed some strange behavior for example sometimes
> "video-hidestats" is logged under "selection" even if the context menu is
> clearly "media" eg:
> 
> "contextmenu": {
> 		"__DEFAULT__": {
> 			"media": {
> 				"withoutcustom": {
> 					"media-playbackrate-050x": 1,
> 					"video-showstats": 2,
> 					"video-hidestats": 1
> 				}
> 			},
> 			"selection": {
> 				"withoutcustom": {
> 					"copy": 1,
> 					"searchselect": 1,
> 					"video-hidestats": 1,
> 					"close-without-interaction": 4,
> 					"video-showstats": 1,
> 					"inspect": 1
> 				}
> 			},
> 			"other": {
> 				"withcustom": {
> 					"custom-page-item": 1
> 				}
> 			}
> 		}
> 	} 
> also I've noticed that some interactions are logged as
> "close-without-interaction" even if it's not the case.

Please file bugs, ideally with more detailed steps to reproduce. Are you sure you're not inadvertently selecting the media as well? Does the context menu show copy/paste?

Comment 44

[Blake Winton (:bwinton) (:☕️)]

I've also seen oddities in the data, so please cc me on the new bug!

(You can see some strangeness at https://bwinton.github.io/d3Experiments/contextmenu.html#
like 36 "paste"s when you're on an image-link, or a "saveimage" when you're on an input…)

Comment 45

[:Gijs (he/him)]

(In reply to Blake Winton (:bwinton) from comment #44)
> I've also seen oddities in the data, so please cc me on the new bug!
> 
> (You can see some strangeness at
> https://bwinton.github.io/d3Experiments/contextmenu.html#
> like 36 "paste"s when you're on an image-link, or a "saveimage" when you're
> on an input…)

As I tried to point out originally (comment 3), the context menu knows about 'state' (which consists of various booleans, ie not mutually exclusive), and the patch was forced to deal with this and turning it into something more like 'type', ie something not mutually exclusive. I would expect these issues if given sufficiently creative HTML, like, say, contenteditable (rich) inputs that have images in them, or such.

I'd really appreciate concrete testcases with what should have happened and what did happen. The fact that some results may be counterintuitive was, in my mind, somewhat expected. :-)

Comment 46

[Catalin Varga [QA][:VarCat]]

Hi I've logged a bug that describes the behavior, please check bug 1100914.