Closed Bug 1016944 Opened 8 years ago Closed 8 years ago

Events are processed twice in BrowserElementPanning.js for nested in-process mozbrowser iframe

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: vingtetun, Assigned: vingtetun)

References

Details

Attachments

(1 file, 1 obsolete file)

Summary: Events are processed twice for nested in-process mozbrowser iframe → Events are processed twice in BrowserElementPanning.js for nested in-process mozbrowser iframe
Fabrice, this patch ensure that the events are processed by the correct BrowserElementPanning.js script related to the event target. Otherwise events are processed by the embedder BEP.js script as well as the one from the embedded content.
Attachment #8430014 - Flags: review?(fabrice)
Comment on attachment 8430014 [details] [diff] [review]
use.deepest.iframes.for.mozbrowser.patch

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

::: dom/browser-element/BrowserElementPanning.js
@@ +90,5 @@
>        return;
>      }
>  
> +    // For nested in-process <iframe mozbrowser> the events are handled
> +    // by the deepest <iframe mozbrowser>.

This comment is not super clear. We could think that the <iframe mozbrowsre> case is already dealt with by the previous test.

@@ +92,5 @@
>  
> +    // For nested in-process <iframe mozbrowser> the events are handled
> +    // by the deepest <iframe mozbrowser>.
> +    var targetWindow = evt.target.ownerDocument.defaultView;
> +    var frameElement = targetWindow.frameElement;

s/var/let

@@ +96,5 @@
> +    var frameElement = targetWindow.frameElement;
> +    while (frameElement) {
> +      targetWindow = frameElement.ownerDocument.defaultView;
> +      frameElement = targetWindow.frameElement;
> +    }

which guarantee do we have to not loop forever?
are we sure that we never have frameElement.ownerDocument.defaultView.frameElement == frameElement ?
Attachment #8430014 - Flags: review?(fabrice)
Assignee: nobody → 21
Attachment #8430014 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8447926 - Flags: review?(fabrice)
(In reply to Fabrice Desré [:fabrice] from comment #2)
> Comment on attachment 8430014 [details] [diff] [review]
> use.deepest.iframes.for.mozbrowser.patch
> 
> Review of attachment 8430014 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> @@ +96,5 @@
> > +    var frameElement = targetWindow.frameElement;
> > +    while (frameElement) {
> > +      targetWindow = frameElement.ownerDocument.defaultView;
> > +      frameElement = targetWindow.frameElement;
> > +    }
> 
> which guarantee do we have to not loop forever?
> are we sure that we never have
> frameElement.ownerDocument.defaultView.frameElement == frameElement ?

The spec at https://developer.mozilla.org/fr/docs/Web/API/window.frameElement says that window.frameElement === null if the element is top level.
Attachment #8447926 - Flags: review?(fabrice) → review+
https://hg.mozilla.org/mozilla-central/rev/5e3f42130258
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.