347731 - extensions/spatialnavigation doesn't build with libxul and bsmedberg's latest landing

Status: RESOLVED DUPLICATE of bug 436084

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Oleg Romashin (:romaxa)]

romaxa@romaxa-sr:/home/work/xulrunner/maemo-browser/build-tree/mozilla/debug-i686-pc-linux-gnu/extensions/spatialnavigation$ make -s
Creating .deps
Creating ../../../dist/include/snav
Creating ../../../dist/xpi-stage/snav
Creating _xpidlgen/.done
nsISpatialNavigation.idl
Creating .deps
Creating ../../../dist/xpi-stage/snav/platform/Linux_x86-gcc3
nsISpatialNavigation.idl
nsSpatialNavigation.cpp
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp: In member function 'virtual nsresult nsSpatialNavigation::KeyPress(nsIDOMEvent*)':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:196: warning: comparison between signed and unsigned integer expressions
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:204: warning: comparison between signed and unsigned integer expressions
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:211: warning: comparison between signed and unsigned integer expressions
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:230: warning: comparison between signed and unsigned integer expressions
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp: In member function 'nsresult nsSpatialNavigation::handleMove(int)':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:637: warning: unused variable 'subdocPresContext'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigation.cpp:539: warning: unused variable 'type'
nsSpatialNavigationUtils.cpp
nsSpatialNavigationService.cpp
nsRect.cpp
nsFont.cpp
nsSpatialNavigation.o: In function `nsCOMPtr<nsIBidirectionalEnumerator>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigation.o: In function `nsCOMPtr<nsIDOMHTMLCollection>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigation.o: In function `nsCOMPtr<nsIDOMEventGroup>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigation.o: In function `nsCOMPtr<nsIDOM3EventTarget>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigation.o: In function `nsCOMPtr<nsIDOMEventTarget>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigation.o:../../../dist/include/xpcom/nsCOMPtr.h:1232: more undefined references to `nsQueryInterface::operator()(nsID const&, void**) const' follow
nsSpatialNavigation.o: In function `~nsString':
../../../dist/include/string/nsTString.h:51: undefined reference to `nsAString_internal::~nsAString_internal()'
nsSpatialNavigation.o: In function `nsCOMPtr<nsIPrefBranch>::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1272: undefined reference to `nsGetServiceByContractID::operator()(nsID const&, void**) const'
nsSpatialNavigationUtils.o: In function `isTargetable(int, nsIFrame*)':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/extensions/spatialnavigation/src/nsSpatialNavigationUtils.cpp:411: undefined reference to `nsString::EqualsIgnoreCase(char const*, int) const'
nsSpatialNavigationUtils.o: In function `nsCOMPtr<nsIDOMAbstractView>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationUtils.o: In function `nsCOMPtr<nsIDOMDocumentView>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationUtils.o: In function `nsCOMPtr<nsIDOMHTMLInputElement>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationUtils.o: In function `nsCOMPtr<nsIDOMHTMLOptionElement>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationUtils.o: In function `nsCOMPtr<nsIDOMHTMLSelectElement>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationUtils.o:../../../dist/include/xpcom/nsCOMPtr.h:1232: more undefined references to `nsQueryInterface::operator()(nsID const&, void**) const' follow
nsSpatialNavigationUtils.o: In function `NS_ConvertUTF16toUTF8':
../../../dist/include/string/nsString.h:158: undefined reference to `AppendUTF16toUTF8(nsAString_internal const&, nsACString_internal&)'
nsSpatialNavigationUtils.o: In function `~nsCString':
../../../dist/include/string/nsTString.h:51: undefined reference to `nsACString_internal::~nsACString_internal()'
nsSpatialNavigationService.o: In function `nsXPIDLCString':
../../../dist/include/string/nsTString.h:582: undefined reference to `nsCharTraits<char>::sEmptyBuffer'
nsSpatialNavigationService.o: In function `nsCOMPtr<nsIServiceManager>::assign_from_qi_with_error(nsQueryInterfaceWithError const&, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1242: undefined reference to `nsQueryInterfaceWithError::operator()(nsID const&, void**) const'
nsSpatialNavigationService.o: In function `nsCOMPtr<nsICategoryManager>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationService.o: In function `nsCOMPtr<nsIPrefBranch>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
nsSpatialNavigationService.o: In function `nsCOMPtr<nsIPrefBranch2>::assign_from_gs_contractid_with_error(nsGetServiceByContractIDWithError const&, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1282: undefined reference to `nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const'
nsSpatialNavigationService.o: In function `nsCOMPtr<nsIWindowWatcher>::assign_from_gs_contractid_with_error(nsGetServiceByContractIDWithError const&, nsID const&)':
../../../dist/include/xpcom/nsCOMPtr.h:1282: undefined reference to `nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const'
nsSpatialNavigationService.o: In function `NS_ConvertUTF16toUTF8':
../../../dist/include/string/nsString.h:147: undefined reference to `AppendUTF16toUTF8(unsigned short const*, nsACString_internal&)'
nsRect.o: In function `operator<<(_IO_FILE*, nsRect const&)':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsRect.cpp:215: undefined reference to `nsString::AppendFloat(float)'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsRect.cpp:217: undefined reference to `nsString::AppendFloat(float)'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsRect.cpp:219: undefined reference to `nsString::AppendFloat(float)'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsRect.cpp:221: undefined reference to `nsString::AppendFloat(float)'
nsRect.o: In function `nsAString_internal::AppendASCII(char const*, unsigned int)':
../../../dist/include/string/nsTSubstring.h:368: undefined reference to `nsAString_internal::ReplaceASCII(unsigned int, unsigned int, char const*, unsigned int)'
nsRect.o: In function `NS_LossyConvertUTF16toASCII':
../../../dist/include/string/nsString.h:103: undefined reference to `LossyAppendUTF16toASCII(nsAString_internal const&, nsACString_internal&)'
nsFont.o: In function `nsFont::EnumerateFamilies(int (*)(nsString const&, int, void*), void*) const':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsFont.cpp:176: undefined reference to `nsString::CompressWhitespace(int, int)'
nsFont.o: In function `nsFont::Equals(nsFont const&) const':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsFont.cpp:98: undefined reference to `nsAString_internal::Equals(nsAString_internal const&, nsStringComparator const&) const'
nsFont.o: In function `nsFont':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsFont.cpp:47: undefined reference to `IsASCII(nsACString_internal const&)'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsFont.cpp:49: undefined reference to `nsAString_internal::AssignASCII(char const*)'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsFont.cpp:47: undefined reference to `IsASCII(nsACString_internal const&)'
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/debug-i686-pc-linux-gnu/extensions/spatialnavigation/src/nsFont.cpp:49: undefined reference to `nsAString_internal::AssignASCII(char const*)'
nsFont.o: In function `nsAString_internal':
../../../dist/include/string/nsTSubstring.h:534: undefined reference to `nsCharTraits<unsigned short>::sEmptyBuffer'
nsFont.o: In function `int nsAString_internal::LowerCaseEqualsLiteral<10>(char const (&) [10]) const':
../../../dist/include/string/nsTSubstring.h:295: undefined reference to `nsAString_internal::LowerCaseEqualsASCII(char const*, unsigned int) const'
nsFont.o: In function `int nsAString_internal::LowerCaseEqualsLiteral<8>(char const (&) [8]) const':
../../../dist/include/string/nsTSubstring.h:295: undefined reference to `nsAString_internal::LowerCaseEqualsASCII(char const*, unsigned int) const'
nsFont.o: In function `int nsAString_internal::LowerCaseEqualsLiteral<6>(char const (&) [6]) const':
../../../dist/include/string/nsTSubstring.h:295: undefined reference to `nsAString_internal::LowerCaseEqualsASCII(char const*, unsigned int) const'
nsFont.o: In function `int nsAString_internal::LowerCaseEqualsLiteral<11>(char const (&) [11]) const':
../../../dist/include/string/nsTSubstring.h:295: undefined reference to `nsAString_internal::LowerCaseEqualsASCII(char const*, unsigned int) const'
nsFont.o: In function `nsAutoString::operator=(nsAString_internal const&)':
../../../dist/include/string/nsTString.h:552: undefined reference to `nsAString_internal::Assign(nsAString_internal const&)'
nsFont.o: In function `nsString::operator=(nsString const&)':
../../../dist/include/string/nsTString.h:106: undefined reference to `nsAString_internal::Assign(nsAString_internal const&)'
nsFont.o: In function `nsString':
../../../dist/include/string/nsTString.h:82: undefined reference to `nsAString_internal::Assign(nsAString_internal const&)'
nsFont.o: In function `~nsString':
../../../dist/include/string/nsTString.h:51: undefined reference to `nsAString_internal::~nsAString_internal()'
nsFont.o: In function `~nsDependentSubstring':
../../../dist/include/string/nsTDependentSubstring.h:45: undefined reference to `nsAString_internal::~nsAString_internal()'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `nsCaseInsensitiveStringComparator::operator()(unsigned short const*, unsigned short const*, unsigned int) const':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/intl/unicharutil/util/internal/nsUnicharUtils.cpp:178: undefined reference to `nsDefaultStringComparator::operator()(unsigned short const*, unsigned short const*, unsigned int) const'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `ToUpperCase(nsAString_internal const&, nsAString_internal&)':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/intl/unicharutil/util/internal/nsUnicharUtils.cpp:162: undefined reference to `nsAString_internal::Assign(nsAString_internal const&)'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `ToLowerCase(nsAString_internal const&, nsAString_internal&)':
/home/work/xulrunner/maemo-browser/build-tree/mozilla-cvs-20060803.0/intl/unicharutil/util/internal/nsUnicharUtils.cpp:130: undefined reference to `nsAString_internal::Assign(nsAString_internal const&)'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `nsDefaultStringComparator':
../../../../dist/include/string/nsTAString.h:68: undefined reference to `vtable for nsDefaultStringComparator'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `nsCOMPtr<nsIObserverService>::assign_from_gs_contractid_with_error(nsGetServiceByContractIDWithError const&, nsID const&)':
../../../../dist/include/xpcom/nsCOMPtr.h:1282: undefined reference to `nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `nsCOMPtr<nsIObserver>::assign_from_qi(nsQueryInterface, nsID const&)':
../../../../dist/include/xpcom/nsCOMPtr.h:1232: undefined reference to `nsQueryInterface::operator()(nsID const&, void**) const'
../../../dist/lib/libunicharutil_s.a(nsUnicharUtils.o): In function `nsAString_internal::BeginWriting()':
../../../../dist/include/string/nsTSubstring.h:122: undefined reference to `nsAString_internal::EnsureMutable(unsigned int)'
collect2: ld returned 1 exit status
make[2]: *** [libsnav.so] Error 1
make[1]: *** [libs] Error 2
make: *** [all] Error 2

Comment 1

[Oleg Romashin (:romaxa)]

Attachment: Possible fix of spatialnavigation

Also there are some fixes in other components that included in spatialnavigation.

Comment 2

[Benjamin Smedberg]

Comment on attachment 245755 [details] [diff] [review]
Possible fix of spatialnavigation

>Index: content/base/src/nsContentList.h

>   /**
>    * Closure data to pass to mFunc when we call it
>    */
>+#ifdef MOZILLA_INTERNAL_API
>   const nsAFlatString* mData;
>+#else
>+  const nsString* mData;
>+#endif

There is no need for this: you can safely use nsAString* since all strings are flat strings now.

And one of the content peers needs to review this change.

>Index: gfx/src/nsFont.cpp

> nsFont::nsFont(const char* aName, PRUint8 aStyle, PRUint8 aVariant,
>                PRUint16 aWeight, PRUint8 aDecoration, nscoord aSize,
>                float aSizeAdjust)
> {
>   NS_ASSERTION(aName && IsASCII(nsDependentCString(aName)),
>                "Must only pass ASCII names here");
>+#ifdef MOZILLA_INTERNAL_API
>   name.AssignASCII(aName);
>+#else
>+  CopyASCIItoUTF16(NS_LITERAL_CSTRING(aName), name);
>+#endif

Why would we ever compile this code in frozen-linkage mode? The GFX changes need to be reviewed by roc or vlad.

Comment 3

[Oleg Romashin (:romaxa)]

> There is no need for this: you can safely use nsAString* since all strings are flat strings now.

Ok, I will fix it.

But what about this:
nsStringKey(const nsAFlatString& str);
nsStringKey(const nsString& str);

? Can I drop it?
nsStringKey(const nsAFlatString& str);

Comment 4

[Oleg Romashin (:romaxa)]

Attachment: Updated patch.

Comment 5

[Benjamin Smedberg]

Comment on attachment 245887 [details] [diff] [review]
Updated patch.

dbaron needs to review this as well

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why all the changes to include nsStringGlue.h from within layout?

Comment 7

[Oleg Romashin (:romaxa)]

> Why all the changes to include nsStringGlue.h from within layout?

hmmm... because nsString.h cannot be included without MOZILLA_INTERNAL_API define,
and without this define should used nsStringAPI.h...

nsStringGlue.h already contain this #ifdef.

Comment 8

[Oleg Romashin (:romaxa)]

Attachment: nsCString -> nsACString fix

- nsPresContext::UpdateCharSet(const nsAFlatCString& aCharSet)
+ nsPresContext::UpdateCharSet(const nsCString& aCharSet)

changed to 

- nsPresContext::UpdateCharSet(const nsAFlatCString& aCharSet)
+ nsPresContext::UpdateCharSet(const nsACString& aCharSet)

Comment 9

[timeless]

Comment on attachment 247536 [details] [diff] [review]
nsCString -> nsACString fix

carrying bsmedberg's r, and rerequesting dbaron's sr...

Comment 10

[Oleg Romashin (:romaxa)]

Comment on attachment 247536 [details] [diff] [review]
nsCString -> nsACString fix

nsPresContext.cpp: In member function 'void nsPresContext::UpdateCharSet(const nsACString_internal&)':
nsPresContext.cpp:817: error: 'const class nsACString_internal' has no member named 'get'
nsPresContext.cpp:851: error: invalid initialization of reference of type 'const nsCString&' from expression of type 'const nsACString_internal'
nsPresContext.cpp:132: error: in passing argument 1 of 'PRBool IsVisualCharset(const nsCString&)'
make[5]: *** [nsPresContext.o] Error 1

Comment 11

[Oleg Romashin (:romaxa)]

Comment on attachment 245887 [details] [diff] [review]
Updated patch.

This works ok

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So, at a quick glance, this patch seems problematic.  Assuming that it really is appropriate to use nsStringGlue.h within the app (which I suspect is true, given bsmedberg's review+), I'm still worried that:

 * converting from nsAFlatString to nsAString will result in loss of performance -- you probably want to convert nsAFlatString to nsString, which is what it's typedef'd to (or is that typedef accessible and it's still a problem?), both in nsContentList and nsHashtable

 * making some of these things usable might encourage people to use them as public interfaces, which we may not want to do

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Also, lists of CPPSRCS, etc. in Makefile.in files are generally indented by two *tabs*.  REQUIRES lines are generally indented by two tabs *and* two spaces, although if you want to use only two tabs that's ok with me.

Comment 14

[Benjamin Smedberg]

FWIW I've been using two spaces for all new makefiles, and only using tabs when required (to indent rules).

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 245887 [details] [diff] [review]
Updated patch.

Marking review- since questions have not been answered; feel free to re-request (maybe on a new patch, if needed) if you answer them.

Comment 17

[Doug Turner (:dougt)]

antonio, romaxa,
Can one of you update the patch to address dbaron's comments?  there are other patches that are blocked from landing until we can compile this extension on the trunk.  If you don't have time, please let me know or reassign.

Comment 18

[Oleg Romashin (:romaxa)]

Attachment: First commit

According David comment, we have to remove internal API usage from spatial navigation...

It means:
1) Frames Rect should be received without nsIFrame.h  (we can use GetBoundingClientRect bug 390426)

2) Disable using nsFrameTraversal.h and nsFrameTraversal creature...
  May be it possible to do by enumerating DOM elements?

This patch can be applied without any problems..., it is not full fix yet.

Comment 19

[Benjamin Smedberg]

Comment on attachment 278217 [details] [diff] [review]
First commit

I don't understand what's going on with nsRect... you're still using it, correct? Why do you no longer need to compile your own copy of nsRect.cpp?

Comment 20

[Benjamin Smedberg]

Comment on attachment 278217 [details] [diff] [review]
First commit

Clearing review until question is answered.

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 278217 [details] [diff] [review]
First commit

Given that bsmedberg cleared the review request to him pending questions, I'll do the same for the one to me.

Comment 22

[Antonio Gomes (tonikitoo)]

finally, taking the bug.

Comment 23

[Antonio Gomes (tonikitoo)]

Attachment: Build snav on trunk using MOZILLA_INTERNAL_API

first version of patch to make spatial navigation work on the trunk. Apply-able with:

$ mozilla
$ patch -p1 < BMO347731_v1.diff

that make snav to work on Firefox. Next steps

1) try moving to non-mozilla-internal-api approach (?)
2) make snav a xulrunner component, not an extension (?)
3) make it work on fennec

Comment 24

[Antonio Gomes (tonikitoo)]

Attachment: Build snav on trunk using MOZILLA_INTERNAL_API v2

same as previous patch 322649 but w/ better identation

Comment 25

[Antonio Gomes (tonikitoo)]

Attachment: Build snav as part of libxul

patch:

* adds MOZ_SNAV in configure.in (and some further stuff there, as well as some changes in /toolkit to make snav to build as part of libxul)
* fixes snav build on trunk (currently 1.9.x tree - not mozilla-central) - still uses MOZILLA_INTERNAL_API
* removes all XPI stuff from its source
* make some identation better.

Comment 26

[Doug Turner (:dougt)]

Comment on attachment 323987 [details] [diff] [review]
Build snav as part of libxul

remove all of the minimo stuff

if you are going to remove building nsRect and nsFont, lets remove the comment.

if you are building this statically, why do you need the nsStringGlue.h include

Comment 27

[Antonio Gomes (tonikitoo)]

Attachment: build snav as part of libxul (right patch uploaded now)

err ... completely wrong patch uploaded =/ sorry about that. doutg, this one the right patch, which addresses already all our comments above.

carrying dougt to review, before bsmedberg.

patch:

* adds MOZ_SNAV in configure.in (and some further stuff there, as well as some
changes in /toolkit to make snav to build as part of libxul)
* fixes snav build on trunk (currently 1.9.x tree - not mozilla-central) -
still uses MOZILLA_INTERNAL_API
* removes all XPI stuff from its source
* make some identation better.

Comment 28

[Antonio Gomes (tonikitoo)]

Attachment: updated patch: cleaned up MINIMO and XPI stuff and added some libxul stuff

patch is against CVS tree (once hg has no /extensions/spatialnavigation dir) - at some point it will have to be moved over hg.

Comment 29

[Antonio Gomes (tonikitoo)]

Attachment: build part of the patch snav

build part of the patch snav (above). 

I separated it because maybe it will be review by someone else apart from dougt. Maybe smedberg ?

Comment 30

[Doug Turner (:dougt)]

Comment on attachment 324998 [details] [diff] [review]
updated patch: cleaned up MINIMO and XPI stuff and added some libxul stuff

What is this about?

+ifdef GNU_CC
+CFLAGS   += -fno-unit-at-a-time
+CXXFLAGS += -fno-unit-at-a-time
+endif

the const char* pref should be using an auto string.

Same with the previous string.

Could you fix that up?

Comment 31

[Antonio Gomes (tonikitoo)]

Attachment: cleaned up MINIMO and XPI stuff and added some libxul stuff (v2)

partually updated according to dougt's comment: remove not needed CC flags from src/Makefile.in 

dougt: I do not see how could we benefit from user "auto string" instead of "const char *", could you explain so that can patch it again ...

ps: patch appliable witg "-p1" , and against 1.9 .

Comment 32

[Antonio Gomes (tonikitoo)]

Comment on attachment 325560 [details] [diff] [review]
cleaned up MINIMO and XPI stuff and added some libxul stuff  (v2)

carrying dougt's review for the snav specific part of the patch

Comment 33

[Doug Turner (:dougt)]

i have combined these two patches into one single patch against hg in bug 436084, I am also cleaning up the patch, including other fixes I know about, and going to ask for a complete review to land.

marking this bug as dup' of 436084.  Lets track again one bug.