Move the platform specific geolocation files

RESOLVED FIXED in mozilla2.0b2

Status

()

RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: MikeK, Assigned: MikeK)

Tracking

Trunk
mozilla2.0b2
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Assignee)

Description

9 years ago
We are creating a new dom/system directory for system specific code, the system specific geolocation code should be moved to this place.
(Assignee)

Comment 1

9 years ago
Created attachment 448821 [details] [diff] [review]
Part 1/4 Movement of win mobile specific files

Patch depends on the patch series in bug 562181
Attachment #448821 - Flags: review?(dougt)
(Assignee)

Comment 2

9 years ago
Created attachment 448822 [details] [diff] [review]
Part 2/4 Movement of maemo specific files
(Assignee)

Updated

9 years ago
Attachment #448822 - Flags: review?(dougt)
(Assignee)

Comment 3

9 years ago
Created attachment 448823 [details] [diff] [review]
Part 3/4 Movement of shared files
(Assignee)

Comment 4

9 years ago
Created attachment 448824 [details] [diff] [review]
Part 4/4 Movement of interface files
Attachment #448824 - Flags: review?(dougt)
(Assignee)

Updated

9 years ago
Attachment #448823 - Flags: review?(dougt)

Comment 5

9 years ago
the patches do not apply cleanly to m-c.

It might be easier for me to review the patches if you just provided the hg move commands, and then one patch that contains the diffs to get everything to build again.

Updated

9 years ago
Attachment #448821 - Flags: review?(dougt) → review-

Updated

9 years ago
Attachment #448822 - Flags: review?(dougt) → review-

Updated

9 years ago
Attachment #448823 - Flags: review?(dougt) → review-

Updated

9 years ago
Attachment #448824 - Flags: review?(dougt) → review-
(Assignee)

Comment 6

9 years ago
Did you apply the patches from bug 562181 before you realized they didn't apply cleanly?

Your wish is my command- I'll reshuffle the content of the patches providing two new patches, one that moves the files, and one that contains the rest of the changes, and update it to trunk if needed.
(Assignee)

Comment 7

9 years ago
Ahh... now I understand what you mean - like:
hg mv xxxx zzzz
hg mv yyyy tttt

and then a patch....

np - will do
(Assignee)

Comment 8

9 years ago
Created attachment 452324 [details] [diff] [review]
Updated to trunk as of this morning

Mark, can you have a look at this with Android eyes? - and if you or someone else could try to do an Android compile, then I can fix any fatal issues before it hits the tree...
Attachment #448821 - Attachment is obsolete: true
Attachment #448822 - Attachment is obsolete: true
Attachment #448823 - Attachment is obsolete: true
Attachment #448824 - Attachment is obsolete: true
Attachment #452324 - Flags: feedback?(mark.finkle)
Comment on attachment 452324 [details] [diff] [review]
Updated to trunk as of this morning


>diff --git a/dom/src/geolocation/Makefile.in b/dom/src/geolocation/Makefile.in
> ifdef WINCE_WINDOWS_MOBILE
>-CPPSRCS         += WinMobileLocationProvider.cpp nsGeoPosition.cpp
>+CPPSRCS         += nsGeoPosition.cpp
>+LOCAL_INCLUDES  += -I$(topsrcdir)/dom/system/android \

Do you want "/dom/system/android" for Windows Mobile?

I don't have an Android build environment yet. Passing to Brad for that part.
Attachment #452324 - Flags: feedback?(blassey.bugs)
Comment on attachment 452324 [details] [diff] [review]
Updated to trunk as of this morning

Looks ok in general
Attachment #452324 - Flags: feedback?(mark.finkle) → feedback+
(note that the prompt interface is going to change when e10s lands)
(Assignee)

Comment 12

9 years ago
Patch depends on the patch in 562181
Depends on: 562181
(Assignee)

Comment 13

9 years ago
(In reply to comment #9)
> 
> Do you want "/dom/system/android" for Windows Mobile?
> 
Probably not, will post an updated version
(Assignee)

Comment 14

8 years ago
Created attachment 453048 [details] [diff] [review]
Updated to trunk and fixed the android instead of windows include issue
Comment on attachment 452324 [details] [diff] [review]
Updated to trunk as of this morning

neither this patch, the updated patch or the patch this bug depends on apply cleanly to trunk, so I can't build with it which I'm assuming is what finkle was looking for. The only thing that jumps out at me is the android include path in the windows mobile ifdef that mfinkle already pointed out.
Attachment #452324 - Flags: feedback?(blassey.bugs)
(Assignee)

Updated

8 years ago
Attachment #452324 - Attachment is obsolete: true
(Assignee)

Comment 16

8 years ago
The patch that this bug depends on has been updated to trunk, this should allow the latest patch here to apply cleanly.
Comment on attachment 453048 [details] [diff] [review]
Updated to trunk and fixed the android instead of windows include issue

with the updated patch on bug 562181 this applies and seems to work fine
Attachment #453048 - Flags: feedback+
(Assignee)

Comment 18

8 years ago
Thank you for trying it out - will move this bug to next stage after the patch in bug 562181 has landed
(Assignee)

Updated

8 years ago
Attachment #453048 - Flags: review?(doug.turner)

Updated

8 years ago
Attachment #453048 - Flags: review?(doug.turner) → review+
Comment on attachment 453048 [details] [diff] [review]
Updated to trunk and fixed the android instead of windows include issue

> CPPSRCS     = \
>         nsAccelerometerSystem.cpp \
>+	AndroidLocationProvider.cpp \
>         $(NULL)

line up.



>+ifdef MOZ_MAEMO_LIBLOCATION
>+        CPPSRCS         += MaemoLocationProvider.cpp
>+        LOCAL_INCLUDES	+= $(MOZ_PLATFORM_MAEMO_CFLAGS)	\
>+                           -I$(topsrcdir)/dom/src/geolocation \
>+                           $(NULL)	

line up the operators.


>+endif
>+	
> include $(topsrcdir)/config/rules.mk
>\ No newline at end of file

add new line.


>+ifdef WINCE_WINDOWS_MOBILE
>+CPPSRCS         += WinMobileLocationProvider.cpp 
>+
>+LOCAL_INCLUDES	+= -I$(topsrcdir)/dom/src/geolocation \
>+                   $(NULL)
>+endif


line up operators


>         nsIBlocklistService.idl \
>         nsIGIOService.idl \
>         nsIAccelerometer.idl \
>+	nsIGeolocationProvider.idl \
>         $(NULL)

line up.


with ws address r+
note that this should NOT land until e10s lands which is days away.
(Assignee)

Comment 21

8 years ago
Created attachment 454495 [details] [diff] [review]
Updated with review comments

for (int i=0;i<100;i++) printf("Remember to turn on visible white spaces, that it aligns nicely on your monitor doesn't mean that it looks nice to anyone else...\n");
Attachment #453048 - Attachment is obsolete: true
(Assignee)

Comment 22

8 years ago
Created attachment 457423 [details] [diff] [review]
Updated to trunk
Attachment #454495 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Updated

8 years ago
Assignee: nobody → mkristoffersen
http://hg.mozilla.org/mozilla-central/rev/aae7ff3ffc78
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b2

Comment 24

8 years ago
Mercurial spit out this warning when I pulled just a little while ago:

added 115 changesets with 634 changes to 611 files
warning: detected divergent renames of dom/interfaces/geolocation/nsIGeolocationProvider.idl to:
 dom/interfaces/geolocation/nsIGeolocationPrompt.idl
 xpcom/system/nsIGeolocationProvider.idl

Doesn't seem like a problem but just in case it is...
(Assignee)

Comment 25

8 years ago
Any idea what that means?
You need to log in before you can comment on or make changes to this bug.