Last Comment Bug 684627 - Make nsEventStateManager::IsHandlingUserInput() time-limited
: Make nsEventStateManager::IsHandlingUserInput() time-limited
Status: RESOLVED FIXED
[sg:want]
:
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Chris Pearce (:cpearce)
:
Mentors:
Depends on: 884950
Blocks: popups 678994 545812
  Show dependency treegraph
 
Reported: 2011-09-04 15:48 PDT by Chris Pearce (:cpearce)
Modified: 2013-06-19 13:10 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (11.70 KB, patch)
2011-09-04 22:51 PDT, Chris Pearce (:cpearce)
bugs: review-
Details | Diff | Splinter Review
Patch v2 (12.68 KB, patch)
2011-09-06 03:00 PDT, Chris Pearce (:cpearce)
bugs: review+
Details | Diff | Splinter Review

Description Chris Pearce (:cpearce) 2011-09-04 15:48:39 PDT
Requests to make an element the full-screen element (see bug 545812) are only granted in user-generated-event handlers. However we don't want long-running user generated event handlers to be able to request full-screen, as this may be used as a way to make users unaware that they've entered full-screen. For example, in a long running event handler, the user may get up and walk away, and then forget they requested full-screen and get phished when they return.

We propose to achieve this by making nsEventStateManager::IsHandlingUserInput() time-limited. i.e. nsEventStateManager::IsHandlingUserInput() only returns true if it's less than a second since the user-generated event.

We'll want a pref to disable this behaviour in case it causes orange in tests.
Comment 1 Olli Pettay [:smaug] 2011-09-04 15:55:55 PDT
Sounds good :)
Comment 2 Chris Pearce (:cpearce) 2011-09-04 22:51:20 PDT
Created attachment 558228 [details] [diff] [review]
Patch v1
Comment 3 Olli Pettay [:smaug] 2011-09-05 07:25:18 PDT
Comment on attachment 558228 [details] [diff] [review]
Patch v1

># HG changeset patch
># Parent c5f2787b006ce2da959a0ffa10d3eaed33a0b7ae
>Bug 684627 - Add time limit to nsEventStateManager::IsHandlingUserInput(). r=?
>
>diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h
>--- a/content/base/public/nsContentUtils.h
>+++ b/content/base/public/nsContentUtils.h
>@@ -78,16 +78,17 @@ static fp_except_t oldmask = fpsetmask(~
> #include "nsReadableUtils.h"
> #include "mozilla/AutoRestore.h"
> #include "nsINode.h"
> #include "nsHashtable.h"
> #include "nsIDOMNode.h"
> #include "nsHtml5Parser.h"
> #include "nsIFragmentContentSink.h"
> #include "nsMathUtils.h"
>+#include "mozilla/TimeStamp.h"
> 
> struct nsNativeKeyEvent; // Don't include nsINativeKeyBindings.h here: it will force strange compilation error!
> 
> class nsIDOMScriptObjectFactory;
> class nsIXPConnect;
> class nsIContent;
> class nsIDOMKeyEvent;
> class nsIDocument;
>@@ -184,16 +185,17 @@ struct nsShortcutCandidate {
>   PRUint32 mCharCode;
>   PRBool   mIgnoreShift;
> };
> 
> class nsContentUtils
> {
>   friend class nsAutoScriptBlockerSuppressNodeRemoved;
>   typedef mozilla::dom::Element Element;
>+  typedef mozilla::TimeDuration TimeDuration;
> 
> public:
>   static nsresult Init();
> 
>   /**
>    * Get a JSContext from the document's scope object.
>    */
>   static JSContext* GetContextFromDocument(nsIDocument *aDocument);
>@@ -1715,16 +1717,23 @@ public:
> 
>   /**
>    * Returns PR_TRUE if key input is restricted in DOM full-screen mode
>    * to non-alpha-numeric key codes only. This mirrors the
>    * "full-screen-api.key-input-restricted" pref.
>    */
>   static PRBool IsFullScreenKeyInputRestricted();
> 
>+  /**
>+   * Returns the time limit on handling user input before
>+   * nsEventStateManager::IsHandlingUserInput() stops returning PR_TRUE.
>+   * This enables us to detect long running user-generated event handlers.
>+   */
>+  static TimeDuration HandlingUserInputTimeout();
>+
>   static void GetShiftText(nsAString& text);
>   static void GetControlText(nsAString& text);
>   static void GetMetaText(nsAString& text);
>   static void GetAltText(nsAString& text);
>   static void GetModifierSeparatorText(nsAString& text);
> 
>   /**
>    * Returns if aContent has a tabbable subdocument.
>diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp
>--- a/content/base/src/nsContentUtils.cpp
>+++ b/content/base/src/nsContentUtils.cpp
>@@ -196,16 +196,17 @@ static NS_DEFINE_CID(kXTFServiceCID, NS_
> #include "nsChannelPolicy.h"
> #include "nsIContentSecurityPolicy.h"
> #include "nsContentDLF.h"
> #ifdef MOZ_MEDIA
> #include "nsHTMLMediaElement.h"
> #endif
> #include "nsDOMTouchEvent.h"
> #include "nsIScriptElement.h"
>+#include "prdtoa.h"
> 
> #include "mozilla/Preferences.h"
> 
> using namespace mozilla::dom;
> using namespace mozilla::layers;
> using namespace mozilla;
> 
> const char kLoadAsData[] = "loadAsData";
>@@ -314,16 +315,39 @@ EventListenerManagerHashClearEntry(PLDHa
> class nsSameOriginChecker : public nsIChannelEventSink,
>                             public nsIInterfaceRequestor
> {
>   NS_DECL_ISUPPORTS
>   NS_DECL_NSICHANNELEVENTSINK
>   NS_DECL_NSIINTERFACEREQUESTOR
> };
> 
>+static TimeDuration sHandlingInputTimeout;
>+
>+static int
>+HandlingInputTimeoutChanged(const char* aPref, void* aClosure)
>+{
>+  nsAdoptingString value =
>+    Preferences::GetString("dom.event.handling-user-input-time-limit");
>+  double seconds = 1.0;
>+  if (!value.IsEmpty()) {
>+    NS_ConvertUTF16toUTF8 utf8(value);
>+    seconds = NS_MAX<double>(0, PR_strtod(utf8.get(), nsnull));
>+  }
>+  sHandlingInputTimeout = TimeDuration::FromSeconds(seconds);
>+  return 0;

I would use milliseconds in the pref, and in that case you could just read
Int or Uint from preference (with a default value).


> 
>+  Preferences::RegisterCallback(HandlingInputTimeoutChanged,
>+                                "dom.event.handling-user-input-time-limit");
You could even use AddIntVarCache here.


> 
>+  // Time at which we began handling user input.
>+  static TimeStamp mHandlingInputStart;
static variables start with 's', not 'm'
Comment 4 Chris Pearce (:cpearce) 2011-09-06 03:00:58 PDT
Created attachment 558432 [details] [diff] [review]
Patch v2

With review comments addressed...
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2011-09-07 09:30:14 PDT
Comment on attachment 558432 [details] [diff] [review]
Patch v2

>--- a/content/base/src/nsContentUtils.cpp
>+++ b/content/base/src/nsContentUtils.cpp

>+#include "prdtoa.h"

That doesn't seem necessary with this version of the patch; could you remove it again?

Note You need to log in before you can comment on or make changes to this bug.