906072 - Remove Maemo port

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Doug Turner (:dougt)]

Attachment: d7403712a305

Comment 1

[Doug Turner (:dougt)]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

johns, can you review the plugin changes?

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

david, I am removing LookAndFeel::eIntID_MaemoClassic.  The change is mechanical.  My only question is does removing eIntID_MaemoClassic widget/LookAndFeel.h screw up anything?  The value exists in the middle of an enum.  (I am pretty sure we don't have any requirements for binary compat here, but I am not sure)

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

ted, can you look over the build changes.

Comment 4

[Oleg Romashin (:romaxa)]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305


>-         PKG_CHECK_MODULES(LIBCONTENTACTION, contentaction-0.1, _LIB_FOUND=1, _LIB_FOUND=)
Need to double check that contentaction is not available anywhere except N9


>-ifndef MOZ_PLATFORM_MAEMO
> GENERATE_CACHE = 1

Do we use this GENERATE_CACHE anywhere?

>-endif
 
on first look it feels safe, need double check how it builds with embedding framework.

Comment 5

[John Schoenick [:johns]]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

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

::: dom/plugins/ipc/PluginInstanceChild.cpp
@@ -2729,5 @@
>      return true;
>  }
>  
>  bool
>  PluginInstanceChild::AnswerHandleKeyEvent(const nsKeyEvent& aKeyEvent,

Can we just remove this and its Parent functions entirely?

@@ -2763,5 @@
>      return true;
>  }
>  
>  bool
>  PluginInstanceChild::AnswerHandleTextEvent(const nsTextEvent& aEvent,

And this

@@ -3261,1 @@
>      {

This scope can be removed with the ifdefs gone

Comment 6

[Oleg Romashin (:romaxa)]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

>-      dnl ========================================================
>-      dnl = Enable meego libcontentaction
>-      dnl ========================================================
>-      MOZ_ARG_ENABLE_BOOL(meegocontentaction,
>-      [  --enable-meegocontentaction           Enable meegocontentaction support],
>-         MOZ_MEEGOCONTENTACTION=1,
>-         MOZ_MEEGOCONTENTACTION=)
>-
>-      if test -n "$MOZ_MEEGOCONTENTACTION"; then
>-
>-         PKG_CHECK_MODULES(LIBCONTENTACTION, contentaction-0.1, _LIB_FOUND=1, _LIB_FOUND=)
>-         if test "$_LIB_FOUND"; then
>-            MOZ_PLATFORM_MAEMO_LIBS="$MOZ_PLATFORM_MAEMO_LIBS $LIBCONTENTACTION_LIBS"
>-            MOZ_PLATFORM_MAEMO_CFLAGS="$MOZ_PLATFORM_MAEMO_CFLAGS $LIBCONTENTACTION_CFLAGS"
>-            MOZ_ENABLE_CONTENTACTION=1
>-            AC_DEFINE(MOZ_ENABLE_CONTENTACTION)
>-            AC_SUBST(MOZ_ENABLE_CONTENTACTION)
>-         fi
>-      fi

Please move this under Qt ifdefs nearby to MOZ_ENABLE_QTMOBILITY, this is not maemo only library.

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

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

I won't r+ this only because you always forget to address review comments on checkin. :-P Just a few things to fix.

::: nsprpub/configure
@@ -3245,5 @@
> -    fi
> -    if test "$MOZ_PLATFORM_MAEMO" = 6; then
> -        MOZ_THUMB=yes
> -    fi
> -    ;;

The NSPR changes need to go in a separate bug.

::: security/nss/coreconf/Linux.mk
@@ +145,5 @@
>  ZDEFS_FLAG		= -Wl,-z,defs
>  DSO_LDOPTS		+= $(if $(findstring 2.11.90.0.8,$(shell ld -v)),,$(ZDEFS_FLAG))
>  LDFLAGS			+= $(ARCHFLAG)
>  
> +# In scratchbox, we need to use the -rpath-link flag for even the standard system

NSS changes should go in a separate bug as well.

::: toolkit/crashreporter/client/Makefile.in
@@ -55,5 @@
> -# so we have to ship our own.
> -libs:: $(DIST)/bin/crashreporter.crt
> -
> -$(DIST)/bin/crashreporter.crt: $(topsrcdir)/security/nss/lib/ckfw/builtins/certdata.txt certdata2pem.py
> -	$(PYTHON) $(srcdir)/certdata2pem.py < $< > $@

You can remove certdata2pem.py as well, it's only used for this rule.

::: toolkit/crashreporter/client/moz.build
@@ -9,5 @@
>  if CONFIG['OS_TARGET'] != 'Android':
>      PROGRAM = 'crashreporter'
>  # The xpcshell test case here verifies that the CA certificate list
> -if CONFIG['MOZ_ENABLE_GTK'] and CONFIG['MOZ_PLATFORM_MAEMO']:
> -    XPCSHELL_TESTS_MANIFESTS += ['maemo-unit/xpcshell.ini']

Your patch should be removing maemo-unit/ as well.

@@ -36,5 @@
>          'crashreporter_unix_common.cpp',
> -    ]
> -    if CONFIG['MOZ_PLATFORM_MAEMO']:
> -        CPP_SOURCES += [
> -            'crashreporter_maemo_gtk.cpp',

I don't see your patch removing crashreporter_maemo_gtk.cpp, you should do that as well.

Comment 8

[Oleg Romashin (:romaxa)]

Comment on attachment 791307 [details] [diff] [review]
d7403712a305

Please move contentaction to standalone configure check. its going to be used outside of maemo context

Comment 9

[Doug Turner (:dougt)]

Attachment: 0001-Bug-906072-Remove-Maemo-port.-r.patch

Comment 10

[Oleg Romashin (:romaxa)]

Comment on attachment 796753 [details] [diff] [review]
0001-Bug-906072-Remove-Maemo-port.-r.patch

Thanks Doug

Comment 11

[(not currently active) Ted Mielczarek]

Comment on attachment 796753 [details] [diff] [review]
0001-Bug-906072-Remove-Maemo-port.-r.patch

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

RIP Maemo.

::: browser/installer/Makefile.in
@@ -101,4 @@
>  INSTALL_SDK = 1
>  endif
>  
> -GENERATE_CACHE = 1

Is this desirable?

::: configure.in
@@ +4802,5 @@
>         MOZ_ENABLE_QTMOBILITY=1
>         MOZ_QT_CFLAGS="$MOZ_QT_CFLAGS $_QTMOBILITY_CFLAGS"
>         MOZ_QT_LIBS="$MOZ_QT_LIBS $_QTMOBILITY_LIBS"
> +       AC_DEFINE(MOZ_ENABLE_QTMOBILITY)
> +       AC_SUBST(MOZ_ENABLE_QTMOBILITY)

This doesn't seem to have been AC_SUBSTed in the past. Is this necessary?

Comment 12

[Doug Turner (:dougt)]

> Is this desirable?

Doesn't look like it is used anymore.

> This doesn't seem to have been AC_SUBSTed in the past. Is this necessary?

I am following the grain of the code above it.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9d925d047ba5

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9d925d047ba5