Closed
Bug 906072
Opened 12 years ago
Closed 12 years ago
Remove Maemo port
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
FIXED
mozilla26
People
(Reporter: dougt, Assigned: dougt)
References
Details
Attachments
(1 file, 1 obsolete file)
|
182.46 KB,
patch
|
romaxa
:
review+
ted
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #791307 -
Flags: review?(romaxa)
| Assignee | ||
Comment 1•12 years ago
|
||
Comment on attachment 791307 [details] [diff] [review]
d7403712a305
johns, can you review the plugin changes?
Attachment #791307 -
Flags: review?(jschoenick)
| Assignee | ||
Comment 2•12 years ago
|
||
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)
Attachment #791307 -
Flags: review?(dbaron)
| Assignee | ||
Comment 3•12 years ago
|
||
Comment on attachment 791307 [details] [diff] [review]
d7403712a305
ted, can you look over the build changes.
Attachment #791307 -
Flags: review?(ted)
Comment 4•12 years ago
|
||
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.
Attachment #791307 -
Flags: feedback+
Comment 5•12 years ago
|
||
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
Attachment #791307 -
Flags: review?(jschoenick) → review+
Comment 6•12 years ago
|
||
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•12 years ago
|
||
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.
Attachment #791307 -
Flags: review?(ted) → review-
Comment 8•12 years ago
|
||
Comment on attachment 791307 [details] [diff] [review]
d7403712a305
Please move contentaction to standalone configure check. its going to be used outside of maemo context
Attachment #791307 -
Flags: review?(romaxa) → review-
| Assignee | ||
Comment 9•12 years ago
|
||
Attachment #791307 -
Attachment is obsolete: true
Attachment #791307 -
Flags: review?(dbaron)
Attachment #796753 -
Flags: review?(romaxa)
Comment 10•12 years ago
|
||
Comment on attachment 796753 [details] [diff] [review]
0001-Bug-906072-Remove-Maemo-port.-r.patch
Thanks Doug
Attachment #796753 -
Flags: review?(romaxa) → review+
| Assignee | ||
Updated•12 years ago
|
Attachment #796753 -
Flags: review?(ted)
Comment 11•12 years ago
|
||
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?
Attachment #796753 -
Flags: review?(ted) → review+
| Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
| Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
| Assignee | ||
Comment 12•12 years ago
|
||
> 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.
Keywords: checkin-needed
Comment 13•12 years ago
|
||
Flags: in-testsuite-
Keywords: checkin-needed
Comment 14•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in
before you can comment on or make changes to this bug.
Description
•