Closed Bug 879515 Opened 6 years ago Closed 6 years ago

Port GTK2 to GTK3 - dom/plugins fixes

Categories

(Core :: Plug-ins, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: stransky, Assigned: stransky)

References

Details

(Whiteboard: [check linux try build before requesting checkin])

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #627699 +++

GTK+ 3.0 and GNOME 3 are approaching and we should get Firefox ready for them. This bug is about missing pieces in dom/plugins directory.
Attached patch dom/plugins patch (obsolete) — Splinter Review
Katl, can you please check this one? It moves xt plugins to gtk2 only. I'd like to go this way for now. 

The future fix may adjust gtk2bin for gtk3 add/or build plugin-container with gtk2.
Attachment #758234 - Flags: review?(karlt)
Comment on attachment 758234 [details] [diff] [review]
dom/plugins patch

(In reply to Martin Stránský from comment #1)
> Created attachment 758234 [details] [diff] [review]
> dom/plugins patch
> 
> Katl, can you please check this one? It moves xt plugins to gtk2 only. I'd
> like to go this way for now.

That's fine, but https://hg.mozilla.org/mozilla-central/diff/94c819a423ff/widget/gtkxtbin/gtk2xtbin.h added GTK3 specific includes.  What has changed your mind here?

>     if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gtk2':
>         CPP_SOURCES += [
>             'nsPluginNativeWindowGtk2.cpp',
>         ]
>+    elif CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gtk3':
>+        CPP_SOURCES += [
>+            'nsPluginNativeWindowGtk3.cpp',
>+        ]
>     elif CONFIG['MOZ_WIDGET_TOOLKIT'] == 'qt':
>         CPP_SOURCES += [
>             'nsPluginNativeWindowQt.cpp',
>         ]
>     else:
>         CPP_SOURCES += [
>             'nsPluginNativeWindow.cpp',
>         ]

nsPluginNativeWindowGtk3.cpp differs from nsPluginNativeWindow.cpp in that it overrides CallSetWindow() to return NS_ERROR_FAILURE.  I don't know why that is necessary.  Can we just use nsPluginNativeWindow.cpp (as is) for now?
CallSetWindow() doesn't feel like the right place to try to disable plugins.
Attachment #758234 - Flags: review?(karlt)
Attached patch v2Splinter Review
This one should address that.

(In reply to Karl Tomlinson (:karlt) from comment #2)
> > Katl, can you please check this one? It moves xt plugins to gtk2 only. I'd
> > like to go this way for now.
> 
> That's fine, but
> https://hg.mozilla.org/mozilla-central/diff/94c819a423ff/widget/gtkxtbin/
> gtk2xtbin.h added GTK3 specific includes.  What has changed your mind here?

I expected to add plugin support but seems to be more complicated so I'd like to finish the main part ad then work on plugins.

> nsPluginNativeWindowGtk3.cpp differs from nsPluginNativeWindow.cpp in that
> it overrides CallSetWindow() to return NS_ERROR_FAILURE.  I don't know why
> that is necessary.  Can we just use nsPluginNativeWindow.cpp (as is) for now?
> CallSetWindow() doesn't feel like the right place to try to disable plugins.

nsPluginNativeWindow.cpp is fine too. I did nsPluginNativeWindowGtk3.cpp due to some build issues but it's already fixed.
Attachment #758234 - Attachment is obsolete: true
Attachment #758490 - Flags: review?(karlt)
Attachment #758490 - Flags: review?(karlt) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/9aeae1c96b26
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Blocks: 1034064
No longer blocks: 1034064
You need to log in before you can comment on or make changes to this bug.