Last Comment Bug 584217 - Add Meego Touch based filepicker implementation
: Add Meego Touch based filepicker implementation
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Qt (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: mozilla9
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on: 570115 594368
Blocks: 583148
  Show dependency treegraph
 
Reported: 2010-08-03 15:33 PDT by Jan Arne Petersen
Modified: 2011-09-15 07:23 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Meego Touch based filepicker implementation (19.09 KB, patch)
2010-08-03 15:33 PDT, Jan Arne Petersen
no flags Details | Diff | Review
Updated patch for MeegoTouch file picker implementation (24.75 KB, patch)
2010-08-16 15:56 PDT, Stefan Hundhammer
dougt: review-
romaxa: review-
Details | Diff | Review
Updated Patch to current version of API (24.96 KB, patch)
2011-07-22 06:00 PDT, Jeremias Bosch (:jbos)
romaxa: review-
Details | Diff | Review
Updated Patch to current version of API - removed qttracker dependency (24.95 KB, patch)
2011-07-25 03:52 PDT, Jeremias Bosch (:jbos)
no flags Details | Diff | Review
Updated Patch to current version of API - removed qttracker dependency (24.47 KB, patch)
2011-07-25 04:06 PDT, Jeremias Bosch (:jbos)
romaxa: review-
Details | Diff | Review
This is a bit less issued version... (22.50 KB, patch)
2011-07-28 00:30 PDT, Jeremias Bosch (:jbos)
no flags Details | Diff | Review
updated security access (22.91 KB, patch)
2011-08-12 04:29 PDT, Jeremias Bosch (:jbos)
romaxa: review-
Details | Diff | Review
Updated - remove hang (21.20 KB, patch)
2011-08-18 10:28 PDT, Jeremias Bosch (:jbos)
no flags Details | Diff | Review
Move MFilePicker to new file and style fixes (28.09 KB, patch)
2011-08-18 22:24 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Current QFliePicker part, make it less depndend from QFileDialog (13.76 KB, patch)
2011-08-19 23:59 PDT, Oleg Romashin (:romaxa)
florian.haenel: review+
jeremias.bosch: review+
jeremias.bosch: feedback+
Details | Diff | Review
Megoutuch implementation (18.28 KB, patch)
2011-08-20 00:00 PDT, Oleg Romashin (:romaxa)
florian.haenel: review+
jeremias.bosch: review+
Details | Diff | Review
Fixed nits, combined patch (32.21 KB, patch)
2011-09-13 11:26 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review

Description Jan Arne Petersen 2010-08-03 15:33:21 PDT
Created attachment 462546 [details] [diff] [review]
Meego Touch based filepicker implementation
Comment 1 Jan Arne Petersen 2010-08-03 15:38:05 PDT
Currently depends on Bug 570115.
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-08-04 22:04:51 PDT
(In reply to comment #1)
> Currently depends on Bug 570115.

Why?
Comment 3 Jan Arne Petersen 2010-08-04 23:39:12 PDT
(In reply to comment #2)
> > Currently depends on Bug 570115.
> 
> Why?

Just because it is currently based on the changed nsFilePicker API from patch in bug 570115 (nsFilePicker::ShowNative). It could be rebased of course.
Comment 4 Oleg Romashin (:romaxa) 2010-08-05 08:57:36 PDT
Comment on attachment 462546 [details] [diff] [review]
Meego Touch based filepicker implementation

I new includes need to be added to [js/src/]config/system-headers
otherwise it fail compile on scratchbox X86
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2010-08-05 11:47:18 PDT
(In reply to comment #3)
> Just because it is currently based on the changed nsFilePicker API from patch
> in bug 570115 (nsFilePicker::ShowNative). It could be rebased of course.

Ah, OK. I'd like to WONTFIX bug 570115, so just wanted to clarify.
Comment 6 Stefan Hundhammer 2010-08-16 15:56:33 PDT
Created attachment 466458 [details] [diff] [review]
Updated patch for MeegoTouch file picker implementation

This is an updated version of the patch at attachment #462546 [details] [diff] [review] from comment #0.

This one fixes a problem the old one had: Multiple file pickers could be opened by clicking the button that opens it multiple times (but slower than multi-click interval) before the first file picker is opened. In that case, now the existing file picker is raised to the front.
Comment 7 Doug Turner (:dougt) 2010-08-17 13:44:40 PDT
Comment on attachment 466458 [details] [diff] [review]
Updated patch for MeegoTouch file picker implementation


>+MOZ_ENABLE_CONTENTMANAGER = @MOZ_ENABLE_CONTENTMANAGER@

We should just use the MOZ_ENABLE_MEEGOTOUCH #define.
> SHARED_LIBRARY_LIBS = ../xpwidgets/libxpwidgets_s.a
> 
> EXTRA_DSO_LDOPTS = \
> 		$(MOZ_COMPONENT_LIBS) \
> 		-lgkgfx \
> 		-lthebes \
> 		$(MOZ_JS_LIBS) \
> 		$(MOZ_QT_LIBS) \
>+		$(MOZ_PLATFORM_MAEMO_LIBS) \
> 		$(GLIB_LIBS) 


I do not understand this change.


>+// Use Qt Tracker API to get attachment data from tracker:
>+#include <QtTracker/Tracker>
>+#include <QtTracker/ontologies/nie.h>
>+#include <QtTracker/ontologies/nfo.h>
>+
>+using namespace SopranoLive;
Comment 8 Oleg Romashin (:romaxa) 2010-08-17 14:49:41 PDT
Comment on attachment 466458 [details] [diff] [review]
Updated patch for MeegoTouch file picker implementation

>+MOZ_ENABLE_CONTENTMANAGER = @MOZ_ENABLE_CONTENTMANAGER@
yes we are not using separate flags for each meego related library... just use meego6 ifdef

>+ifeq ($(MOZ_ENABLE_CONTENTMANAGER), 1)
>+MOCSRCS += moc_nsFilePicker.cpp
>+endif

This is not meego or maemo specific... it is just Qt object moc-ization.... it will break build if MOZ_ENABLE_CONTENTMANAGER not defined


>+
>+#if HAVE_VISIBILITY_ATTRIBUTE
>+#  define NSFILEPICKER_EXPORT __attribute__ ((visibility ("default")))
>+#else
>+#  define NSFILEPICKER_EXPORT
>+#endif

there are some macros available already in nscore.h, something like NS_VISIBILITY_DEFAULT, NS_EXTERNAL_VIS
I think it is better to use ready macros here.


>+
>+static QString openFilename(QWidget *parent, const QString &caption,

If I remember correctly it should be like this, according to latest mozilla code style:
 +static QString openFilename(QWidget* parent, const QString& caption,

>+{
>+    QString result;
>+
>+    if (MeegoFilePicker::pickerActive()) {
>+        qDebug() << __FUNCTION__  << ": Ignoring request to open secondary content picker";
I don't really like idea of putting qDebug entries everywhere... they are really annoying on desktop builds and put messages to stdout...

See bug 588186



>+
>+    if (picker) { // Need separate 'if' because picker might have been destroyed by its parent during exec()
>+        if ( picker->hasSelectedFileNames()) {
              ^ - some spaces not  needed  style style...
>+            result = picker->selectedFileNames();



> 
>-    mDialog->selectFile(QString::fromUtf16(mDefault.get()));
>+    if (!selected.isEmpty()) {
>+      QString path = QFile::encodeName(selected);
I think the rest of file is using 4 spaces...


>-void nsFilePicker::InitNative(nsIWidget *parent, const nsAString &title, PRInt16 mode)
>+void nsFilePicker::InitNative(nsIWidget *aParent, const nsAString &aTitle,
>+                              PRInt16 aMode)

Is this change really needed ?, remove all style only changes pls


>+MApplicationPage * MeegoFilePicker::createOpenFilePage()
>+{
>+    QStringList itemType("http://www.semanticdesktop.org/ontologies/2007/03/22/nfo#FileDataObject");
this string is used  many times here, I think it should be const....
also is it documented feature? do we have meego define for this string?
Comment 9 Oleg Romashin (:romaxa) 2010-08-17 14:51:36 PDT
Comment on attachment 462546 [details] [diff] [review]
Meego Touch based filepicker implementation

I think this patch is obsolete now...
Comment 10 Jan Arne Petersen 2010-08-17 14:57:19 PDT
Comment on attachment 462546 [details] [diff] [review]
Meego Touch based filepicker implementation

Obsolete.
Comment 11 Jeremias Bosch (:jbos) 2011-07-22 06:00:40 PDT
Created attachment 547673 [details] [diff] [review]
Updated Patch to current version of API

This patch enables the filepicker of Harmattan. For normal Qt the usual Filepicker is still used.
Comment 12 Oleg Romashin (:romaxa) 2011-07-22 11:35:21 PDT
Comment on attachment 547673 [details] [diff] [review]
Updated Patch to current version of API

This patch is not against mozilla-central source code...
> +        MOZ_QT_LIBS="-L$QTDIR/lib/ -lQtGui -lQtNetwork -lQtCore -lQtDBus -lQtXml -lQtOpenGL -lQtSparql"

Sounds like this will force linking against lQtSparql which is not existing on desktop...

># User Stefan Hundhammer <Stefan.Hundhammer@xxx.com>
I guess you reworked this patch , and should paste your name here...
Comment 13 Oleg Romashin (:romaxa) 2011-07-22 15:00:11 PDT
Comment on attachment 547673 [details] [diff] [review]
Updated Patch to current version of API

libqttracker1pre6 - is not available in default image... so this dependency must be removed...
Comment 14 Jeremias Bosch (:jbos) 2011-07-25 03:52:58 PDT
Created attachment 548142 [details] [diff] [review]
Updated Patch to current version of API - removed qttracker dependency
Comment 15 Jeremias Bosch (:jbos) 2011-07-25 04:06:13 PDT
Created attachment 548144 [details] [diff] [review]
Updated Patch to current version of API - removed qttracker dependency

Overlooked #12, updated.
Comment 16 Oleg Romashin (:romaxa) 2011-07-25 11:49:19 PDT
Comment on attachment 548144 [details] [diff] [review]
Updated Patch to current version of API - removed qttracker dependency


>-        MOZ_QT_LIBS="-L$QTDIR/lib/ -lQtGui -lQtNetwork -lQtCore -lQtDBus -lQtXml -lQtOpenGL"
>+        MOZ_QT_LIBS="-L$QTDIR/lib/ -lQtGui -lQtNetwork -lQtCore -lQtDBus -lQtXml -lQtOpenGL" 
Any useful change in here?


>diff -r 2be0b2e30b49 debian/control
>--- a/debian/control	Mon Jul 25 12:55:46 2011 +0200
>+++ b/debian/control	Mon Jul 25 13:02:52 2011 +0200
>@@ -2,7 +2,7 @@
> Section: unknown
> Priority: extra
> Maintainer: Oleg Romashin <oleg.romashin@nokia.com>
>-Build-Depends: debhelper (>= 4),  autoconf2.13, libidl-dev, quilt, python (>= 2.4), libqt4-dev, libxext-dev, libxext-dev, libpng12-dev | libpng-dev, libfontconfig1-dev, libcontentaction-dev, sharutils, aegis-builder, libiw-dev, libdbus-1-dev, libdbus-glib-1-dev, libasound2-dev, libxrender-dev, libpango1.0-dev, yasm, libqtm-sensors-dev, libqmsystem2-dev, libmeegotouch-dev
>+Build-Depends: debhelper (>= 4),  autoconf2.13, libidl-dev, quilt, python (>= 2.4), libqt4-dev, libxext-dev, libxext-dev, libpng12-dev | libpng-dev, libfontconfig1-dev, libcontentaction-dev, sharutils, aegis-builder, libiw-dev, libdbus-1-dev, libdbus-glib-1-dev, libasound2-dev, libxrender-dev, libpango1.0-dev, yasm, libqtm-sensors-dev, libqmsystem2-dev, libmeegotouch-dev, libqtsparql-dev 
> Standards-Version: 3.7.3

this is not part of mozilla-central

> Package: fennec
>diff -r 2be0b2e30b49 extensions/spellcheck/hunspell/tests/suggestiontest/Makefile.orig
>--- a/extensions/spellcheck/hunspell/tests/suggestiontest/Makefile.orig	Mon Jul 25 12:55:46 2011 +0200
>+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
>@@ -1,11 +0,0 @@
>-all:
>-	./prepare
>-	./test
>-
>-single:
>-	./prepare2
>-	./test
>-
>-clean:
>-	rm *.[1-5] result.*
>-

garbage?


>+#include <qfile.h>
>+#include <qstringlist.h>
>+#include <qgraphicswidget.h>

I guess it is better to use Qt header wrappers isn't it? like <QGraphicsWidget>


>+    }
>+
>+    if (picker) { // Need separate 'if' because picker might have been destroyed by its parent during exec()
>+        if ( picker->hasSelectedFileNames()) {
             ^ - indent, random whitespaces...
>+            result = picker->selectedFileNames().first();
>+        }

>+
>+static QString openDirectory(QWidget *parent, const QString &caption, const
                                      
(QWidget* parent - this is more close to moz-style

>-            selected = files[0];
>+    #if defined(MOZ_ENABLE_CONTENTMANAGER) && defined(MOZ_ENABLE_MEEGOTOUCH)
>+        MWindow *parentWidget = MApplication::activeWindow();
>+    #else

ifdefs should start from beginning of the line




>+//---------------
>+#if defined(MOZ_ENABLE_CONTENTMANAGER) && defined(MOZ_ENABLE_MEEGOTOUCH)

Does MOZ_ENABLE_CONTENTMANAGER make any sense without meegotouch?


>+
>+        MWindow * parentWindow = MApplication::activeWindow();

MWindow* parentWindow, here and in other places


Also I still see some strange list of applications, instead of Documents/Photos/Media list.
Comment 17 Jeremias Bosch (:jbos) 2011-07-26 05:28:51 PDT
harmattan always show this strange list of applications... (see search in system grid...) you may use "FileObject" as type - where you see docs, media,... but all emtpy so harmattan does not seem to use that. 

Fixes coming
Comment 18 Oleg Romashin (:romaxa) 2011-07-26 08:21:38 PDT
Problem is that on android we have also list of applications, but you are able to choose that app and select required file and get path into "input file" element.
Also on harmattan Gallery able to get list of images, media player able to get list of media files, and documents app getting list of documents, so obviously it is somehow possible to do that... can we look inside these apps and check how do they do that?

otherwise I would prefer to keep ugly but working/functional Qt styled filepicker rather than nice harmattan styled dialog which does not allow to do anything...
Comment 19 Jeremias Bosch (:jbos) 2011-07-27 06:11:28 PDT
you can still search for content, ie. type mp3 or image...
Comment 20 Oleg Romashin (:romaxa) 2011-07-27 11:21:03 PDT
oh... that is not really obvious... can we just pre-search something like "*.jpg" in search field, so it will find all available files in tracker?
And could you remove non-file types from search results?
Comment 21 Jeremias Bosch (:jbos) 2011-07-27 11:31:27 PDT
jep, actually, we just setup a long presearch (first QString() parameter)

Like mp3 mp4 avi pdf doc ogg webm zip jpg jpeg png bmp   it will show them all
Comment 22 Jeremias Bosch (:jbos) 2011-07-27 11:33:31 PDT
a i forgot, space is separator and the user wont see the search string, though I will push fixed patch with that.

stuff like wildcards do not work...
Comment 23 Jeremias Bosch (:jbos) 2011-07-28 00:30:52 PDT
Created attachment 549040 [details] [diff] [review]
This is a bit less issued version...
Comment 24 Oleg Romashin (:romaxa) 2011-07-28 12:02:27 PDT
Just tested new version and when I selected some file, it was not inserted into upload input field..... 
Also another issue is that when you remove text from search field it show documents/music et.c clicking on them does not do anything
Comment 25 Jeremias Bosch (:jbos) 2011-07-28 12:28:46 PDT
(In reply to comment #24)
> Just tested new version and when I selected some file, it was not inserted
> into upload input field.....
 
You need to sign your packages with aegis... (see the bugreport about that)


> Also another issue is that when you remove text from search field it show
> documents/music et.c clicking on them does not do anything

See... Thats why I did simply the "default" list...
Comment 26 Oleg Romashin (:romaxa) 2011-08-08 15:06:37 PDT
> > into upload input field.....
>  
> You need to sign your packages with aegis... (see the bugreport about that)

I have that patch applied, but still not able to get any text/file path into HTML element...
Comment 27 Jeremias Bosch (:jbos) 2011-08-08 22:59:23 PDT
I checked your build. There its really not working, while for mine it does (same device, image,...)

can you post me your mozconfig? How do you build the things? something must be different.
Comment 28 Oleg Romashin (:romaxa) 2011-08-08 23:58:28 PDT
My mozconfig just that one which is generated by dpkg-buildpackage from hg.meego.com
then make deb -C obj-build...

Also I see stuff implemented here, but it does not work fully... does build from hg.meego.com works for you?
Comment 29 Jeremias Bosch (:jbos) 2011-08-12 04:29:54 PDT
Created attachment 552632 [details] [diff] [review]
updated security access

I looks like the issue was caused through some security system stuff...
With the updated version it works for me now - can you verify?
Comment 30 Oleg Romashin (:romaxa) 2011-08-12 14:32:04 PDT
Comment on attachment 552632 [details] [diff] [review]
updated security access

Ok, I finally got it mostly working

>   <request>
>     <credential name="TrackerReadAccess" />
>     <credential name="TrackerWriteAccess" />
>+    <for path="/usr/local/lib/fennec-8.0a1/fennec" />
Add filter substitution and use @installdir@/@MOZ_APP_NAME@


>+// Extern variables from qfiledialog.cpp deep inside libqt
>+NSFILEPICKER_EXPORT extern _qt_file_open_filename_hook qt_filedialog_open_filename_hook;
>+NSFILEPICKER_EXPORT extern _qt_file_open_filenames_hook qt_filedialog_open_filenames_hook;
>+NSFILEPICKER_EXPORT extern _qt_file_existing_directory_hook qt_filedialog_existing_directory_hook;

Ok, I was looking at this implementation year ago and now and still don't like this hacky approach, and magic with file dialogs...
1) IIUC we are creating normal QFileDialog and then overriding it's view using hacks and displaying ContentManager view on top of it...
   This make stuff unstable, it is easy to get fennec frozen and killed if you move fennec to TS and try to close it in TS.
2) Would not it be better instead of launching QFileDialog and doing terrible hacks on top of it, just create normal Q(GraphicsView)Widget and just integrate Application page into that widget.
3) Also I think it is better to isolate MeegoContent manager impllementation into it's own file (nsMeegoFilePicker or something) and move full implementation into there... including Widget creation et.c

Also don't forget about styling... it seems broken in many places in new implementation
Comment 31 Jeremias Bosch (:jbos) 2011-08-18 10:28:43 PDT
Created attachment 554130 [details] [diff] [review]
Updated - remove hang
Comment 32 Oleg Romashin (:romaxa) 2011-08-18 22:24:16 PDT
Created attachment 554305 [details] [diff] [review]
Move MFilePicker to new file and style fixes

Last patch crashes, but now right when we go to background... 
Here I created mechanically reorganized version of previous patch, no hooks, moved into new file, fixed style issues.
Still need to cleanup list of  (some of them obviously not needed). also need to understand logic of Meegotouch window creation and how to simplify it.
Comment 33 Oleg Romashin (:romaxa) 2011-08-19 23:59:05 PDT
Created attachment 554614 [details] [diff] [review]
Current QFliePicker  part, make it less depndend from QFileDialog
Comment 34 Oleg Romashin (:romaxa) 2011-08-20 00:00:35 PDT
Created attachment 554615 [details] [diff] [review]
Megoutuch implementation
Comment 35 Oleg Romashin (:romaxa) 2011-08-23 00:59:18 PDT
Still need to check that first patch did not break old filepicker behavior, multiselection et.c. on desktop
Comment 36 Oleg Romashin (:romaxa) 2011-08-25 21:06:29 PDT
Comment on attachment 554614 [details] [diff] [review]
Current QFliePicker  part, make it less depndend from QFileDialog

Ok, Multi-file picker seems does not work even with current code...
we should fix that in another bug.
Comment 37 Oleg Romashin (:romaxa) 2011-08-25 21:08:23 PDT
Comment on attachment 554615 [details] [diff] [review]
Megoutuch implementation

MEegotouch implementation, moved into separate file, and ifdefed as replacement for QFileDialog
Comment 38 Wolfgang Rosenauer [:wolfiR] 2011-08-29 05:51:11 PDT
I'm about to leave for vacation for two weeks and cannot review that before anymore. Sorry.
Comment 39 Oleg Romashin (:romaxa) 2011-08-29 09:31:37 PDT
Comment on attachment 554615 [details] [diff] [review]
Megoutuch implementation

Someone with fresh eye should look at this impl...
Comment 40 Jeremias Bosch (:jbos) 2011-09-05 05:36:52 PDT
Comment on attachment 554615 [details] [diff] [review]
Megoutuch implementation

Looks and works fine for me.
Comment 41 Jeremias Bosch (:jbos) 2011-09-05 05:38:40 PDT
Comment on attachment 554614 [details] [diff] [review]
Current QFliePicker  part, make it less depndend from QFileDialog

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

Its fine to do it this way. Works as expected.
Comment 42 Oleg Romashin (:romaxa) 2011-09-13 01:48:35 PDT
Comment on attachment 554615 [details] [diff] [review]
Megoutuch implementation

Florian take a look at this with fresh eyes
Comment 43 Florian Hänel [:heeen] 2011-09-13 04:33:15 PDT
Comment on attachment 554615 [details] [diff] [review]
Megoutuch implementation

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

r+ with issues resolved

::: widget/src/qt/nsMFilePicker.cpp
@@ +129,5 @@
> +{
> +    QStringList itemType("http://www.semanticdesktop.org/ontologies/2007/03/22/nfo#FileDataObject");
> +
> +    SelectSingleContentItemPage* page =
> +        new SelectSingleContentItemPage(QString("pdf mp3 mp4 jpg png avi acc ogg doc zip"),

Do we really need to hardcode this list? Is it really guaranteed to be complete? What about html, txt, xls, rar, tar.gz, mkv, flac, ogv...
Can we just allow for all types and let tracker decide what it serves us?

@@ +239,5 @@
> +    picker->mCaption = caption;
> +    picker->exec();
> +
> +    // Need separate 'if' because picker might have been destroyed by its parent during exec()
> +    if (picker->hasSelectedFileNames()) {

does this mean picker could be NULL here? If so we need to do if(picker && picker->...)

@@ +257,5 @@
> +    picker->mCaption = caption;
> +    picker->exec();
> +
> +    // Need separate 'if' because picker might have been destroyed by its parent during exec()
> +    if (picker->hasSelectedFileNames()) {

see NULL comment above

@@ +278,5 @@
> +    picker->mCaption = caption;
> +    picker->exec();
> +
> +    // Need separate 'if' because picker might have been destroyed by its parent during exec()
> +    if (picker->hasSelectedFileNames()) {

see above
Comment 44 Florian Hänel [:heeen] 2011-09-13 04:41:10 PDT
Comment on attachment 554614 [details] [diff] [review]
Current QFliePicker  part, make it less depndend from QFileDialog

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

r+ with nits resolved

::: widget/src/qt/nsFilePicker.cpp
@@ +238,5 @@
> +
> +    switch (mMode) {
> +    case nsIFilePicker::modeOpen:
> +        selected = MozFileDialog::getOpenFileName(parentQWidget, mCaption,
> +                                                  directory.get(), filters.join(";;"));

I think single ";" should be enough here?

@@ +246,5 @@
> +        }
> +        break;
> +    case nsIFilePicker::modeOpenMultiple:
> +        files = MozFileDialog::getOpenFileNames(parentQWidget, mCaption,
> +                                                directory.get(), filters.join(";;"));

see above
Comment 45 Florian Hänel [:heeen] 2011-09-13 04:57:15 PDT
the second patch is giving me build errors:


invoking make to create js-config script
make[3]: Entering directory `/home/fhae/upstream/mozilla-central/obj-fn-qt-arm/js/src'
make[3]: `js-config' is up to date.
make[3]: Leaving directory `/home/fhae/upstream/mozilla-central/obj-fn-qt-arm/js/src'
config/autoconf.mk is unchanged
make[2]: Leaving directory `/home/fhae/upstream/mozilla-central'
TEST-UNEXPECTED-FAIL | check-sync-dirs.py | build file copies are not in sync
TEST-INFO | check-sync-dirs.py | file(s) found in:               /home/fhae/upstream/mozilla-central/js/src/config
TEST-INFO | check-sync-dirs.py | differ from their originals in: /home/fhae/upstream/mozilla-central/config
TEST-INFO | check-sync-dirs.py | differing file:                 ./system-headers
In general, the files in '/home/fhae/upstream/mozilla-
central/js/src/config' should always be exact copies of originals in
'/home/fhae/upstream/mozilla-central/config'.  A change made to one should
also be made to the other.  See 'check-sync-dirs.py' for more details.
make[1]: *** [realbuild] Error 1
make[1]: Leaving directory `/home/fhae/upstream/mozilla-central'
make: *** [build] Error 2
Comment 46 Oleg Romashin (:romaxa) 2011-09-13 11:26:48 PDT
Created attachment 559998 [details] [diff] [review]
Fixed nits, combined patch
Comment 47 Oleg Romashin (:romaxa) 2011-09-13 19:57:53 PDT
try build looks good enough
https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=79cc02cd81d7
Comment 49 :Ehsan Akhgari (busy, don't ask for review please) 2011-09-15 07:23:35 PDT
https://hg.mozilla.org/mozilla-central/rev/131496f18137

Note You need to log in before you can comment on or make changes to this bug.