435917 - showing the native gtk+ print dialogue causes backward typing in other windows

Status: RESOLVED INACTIVE

(Core :: Widget: Gtk, defect, P3)

Description

[Christian Persch (GNOME) (away; not receiving bug mail)]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008052702 Minefield/3.1a1pre

Steps to reproduce:
0) Open 2 windows and load a web page in each
1) In window 1's URL bar, type "javascript:window.print()" and load the URL. The native gtk print dialogue shows as modal to window 1.
2) In window 2's URL bar, type "www.gnome.org"

Results:
You actually typed "gro.emong.www" in window 2's URL bar!

Same happens if you type in form controls in window 2's web page, if the web page is from a different host than the one in window 1.

Similar to bug 179307, and marking security sensitive b/c of bug 179307 comment 14. That bug also shows how to fix the problem.

The gtk native file chooser dialogue code has the same problem, but I couldn't find a way to show this behaviour since there's no way to pop it up from page JS.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Hilarious. Michael, could you spin a patch? Bug 179307 should make it pretty clear what needs to be done here.

Comment 2

[Michael Ventnor]

Attachment: Patch

Thanks for finding this! This just does what Firefox used to do in XPFE, which was push the JS stack while running the event loop, and I think it's as simple as it's going to get. Would love this in RC2.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 322711 [details] [diff] [review]
Patch

this is OK, but maybe it would be better to push this further up the call stack so that embedders who replace this component don't have to deal with it?

Comment 4

[Michael Ventnor]

(In reply to comment #3)
> (From update of attachment 322711 [details] [diff] [review])
> this is OK, but maybe it would be better to push this further up the call stack
> so that embedders who replace this component don't have to deal with it?
> 

Then we get into cross-platform code. If Windows and Mac don't show this problem we probably should leave it alone at this late stage.

Comment 5

[Michael Ventnor]

Comment on attachment 322711 [details] [diff] [review]
Patch

Would like this small patch for RC2, considering possible security risks.

Comment 6

[Michael Ventnor]

This has a reviewed patch, and the rationale is in the above comments, so I'd really like this for RC2.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #4)
> Then we get into cross-platform code. If Windows and Mac don't show this
> problem we probably should leave it alone at this late stage.

I don't see the problem on Mac because the print dialog is app-modal there.

I would like to see this pushed up as soon as we can handle the risk.

Comment 8

[Michael Ventnor]

I tried pushing it up one level to nsPrintEngine but for some reason it doesn't work there, I can't access the JS stack or push/pop it properly.

Comment 9

[Mike Shaver (:shaver emeritus)]

I can reproduce this on Windows (Vista).

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

What do you mean you "can't access the JS stack"?

Comment 11

[Mike Shaver (:shaver emeritus)]

Not a 3.0 blocker per drivers (really just me and my cat, but this bug is security-sensitive so nobody will ever learn the secret!), good fodder for 3.0.1.

Comment 12

[Michael Ventnor]

(In reply to comment #10)
> What do you mean you "can't access the JS stack"?
> 

nsCOMPtr<nsIJSContextStack> stack(do_GetService("@mozilla.org/js/xpc/ContextStack;1"));

gives null.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

that makes no sense. Try using nsCxPusher from nsContentUtils.h?

Comment 14

[Michael Ventnor]

Attachment: Patch 2

Yeah, this works.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can you add { } scoping so that the stack is popped right after the ShowPrintDialog call? That will reduce the chance of privilege escalation attacks. Otherwise looks great. Should fix Windows too, someone should check that.

Comment 16

[Michael Ventnor]

Comment on attachment 322894 [details] [diff] [review]
Patch 2

Uh, I completely forgot about the actual testcase (I was supposed to do window.print() istead of File>Print).

Turns out this patch doesn't do anything.

Comment 17

[Michael Ventnor]

Comment on attachment 322711 [details] [diff] [review]
Patch

roc, any ideas?

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Using the explicit code, in nsPrintEngine, should work. I don't know why it would return null for the service.

jst, what's the best way to do this?

Comment 19

[Johnny Stenback (:jst)]

That makes no sense to me. This is all main thread, right? But even if it wasn't, I can't see why the service manager would return the service in one piece of code but not the other.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is there a reason why, as designed, nsCxPusher can't push 'null'?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Also Michael, please post your patch for nsPrintEngine.

Comment 22

[Johnny Stenback (:jst)]

(In reply to comment #20)
> Is there a reason why, as designed, nsCxPusher can't push 'null'?

Not necessarily, but in general it's critical if asked to push a context found from a DOM node or what not that a context *is* found and then pushed. Lots of security sensitive code depends on that. The Push() method that takes a JSContext* argument could be permitted to push null I guess, but I don't think we want to change that w/o putting it through a fair bit of use.

Comment 23

[Brandon Sterne (:bsterne)]

Is there any progress here?  There is an updated patch but it hasn't received review yet.  Does it fix the problem?  This could use a severity rating as well.

Comment 24

[Daniel Veditz [:dveditz]]

Un

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

Moving all keyboard/IME open bugs to DOM: UI Events & Focus Handling component.

Comment 27

[BugBot [:suhaib / :marco]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 9/8)]

I'm not sure whether this is a valid bug even in these days, but according to the granted path, the method is now managed in Widget:Gtk.

Comment 29

[Sylvestre Ledru [:Sylvestre]]

No activity for years, closing.
Please reopen if this is still impacting you.