organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues 2.7k Pull requests 182 Projects Releases 65 Wiki Activity Actions

WIP: [android][ios] Unwrap maps.goo.gl short links via a proxy #5386

Draft
kartikaysaxena wants to merge 11 commits from kartikaysaxena/master into master
pull from: kartikaysaxena/master
merge into: organicmaps:master
Conversation 0 Commits 11 Files changed 6 +264 -5
kartikaysaxena commented 2023-06-23 16:09:40 +00:00 (Migrated from github.com)
Copy link

Integrated proxy to the iOS codebase.

Integrated proxy to the iOS codebase.
👍 1
biodranik (Migrated from github.com) reviewed 2023-06-23 16:09:40 +00:00
biodranik (Migrated from github.com) requested changes 2023-06-23 20:37:05 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
biodranik (Migrated from github.com) commented 2023-06-23 20:20:16 +00:00

Why watcher? Isn't it enough to check the pasteboard once on the app foreground activation?

Why watcher? Isn't it enough to check the pasteboard once on the app foreground activation?
biodranik (Migrated from github.com) commented 2023-06-23 20:21:12 +00:00

What kind of urls are supported now by a proxy? Google Maps? Something else? Only these URLs should be explicitly detected in the pasteboard and passed to the proxy.

What kind of urls are supported now by a proxy? Google Maps? Something else? Only these URLs should be explicitly detected in the pasteboard and passed to the proxy.
biodranik (Migrated from github.com) commented 2023-06-23 20:25:07 +00:00
  1. Text should be different, something like "Extracting coordinates from %@url using the anonymous proxy..."
  2. All text translations are stored in data/strings/strings.txt and regenerated using tools/unix/generate_localizations.sh in a separate commit (there is a doc about it).
1. Text should be different, something like "Extracting coordinates from %@url using the anonymous proxy..." 2. All text translations are stored in data/strings/strings.txt and regenerated using tools/unix/generate_localizations.sh in a separate commit (there is a doc about it).
biodranik (Migrated from github.com) commented 2023-06-23 20:27:23 +00:00

Please use a constant for the URL in the beginning of the mm file, it will be changed to https://proxy.omaps.app/coordinates
(why /search btw?)

Please use a constant for the URL in the beginning of the mm file, it will be changed to https://proxy.omaps.app/coordinates (why /search btw?)
biodranik (Migrated from github.com) commented 2023-06-23 20:29:16 +00:00

Proxy request error: %@

Proxy request error: %@
biodranik (Migrated from github.com) commented 2023-06-23 20:30:22 +00:00

"Failed to extract coordinates from %url using an anonymous proxy. Please check your Internet connection"

"Failed to extract coordinates from %url using an anonymous proxy. Please check your Internet connection"
biodranik (Migrated from github.com) commented 2023-06-23 20:32:17 +00:00

Proxy response: %@

Proxy response: %@
biodranik (Migrated from github.com) commented 2023-06-23 20:33:06 +00:00

"Can not extract coordinates from %url"

"Can not extract coordinates from %url"
biodranik (Migrated from github.com) commented 2023-06-23 20:34:39 +00:00

What else can be extracted from Google except for coordinates? Place name? It can be passed via om:// URL scheme, see https://omaps.app/api

What else can be extracted from Google except for coordinates? Place name? It can be passed via om:// URL scheme, see https://omaps.app/api
biodranik (Migrated from github.com) commented 2023-06-23 20:35:31 +00:00

Please pass the title as a parameter and use it appropriately.

Please pass the title as a parameter and use it appropriately.
biodranik (Migrated from github.com) commented 2023-06-23 20:36:10 +00:00

There is already a translation for the OK button in strings.txt, check how to use it in other mm files.

There is already a translation for the OK button in strings.txt, check how to use it in other mm files.
iphone/Maps/Classes/MapsAppDelegate.mm
biodranik (Migrated from github.com) commented 2023-06-23 20:37:00 +00:00

Is watching really needed? Can pasteboard checking be implemented only on the app activation?

Is watching really needed? Can pasteboard checking be implemented only on the app activation?
kartikaysaxena (Migrated from github.com) reviewed 2023-06-24 23:52:31 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-24 23:52:30 +00:00

That's only the name of the implementation, it runs only once whenever someone enters foreground from background (or changes app and even a cold start).

That's only the name of the implementation, it runs only once whenever someone enters foreground from background (or changes app and even a cold start).
kartikaysaxena (Migrated from github.com) reviewed 2023-06-24 23:53:48 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-24 23:53:47 +00:00

Right now Google Maps URIs are supported by the proxy, I have updated the code such that it would make a call only if the host name corresponds to google maps' hostnames.

Right now Google Maps URIs are supported by the proxy, I have updated the code such that it would make a call only if the host name corresponds to google maps' hostnames.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-24 23:54:13 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-24 23:54:13 +00:00

Changed it.

Changed it.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-24 23:56:02 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-24 23:56:02 +00:00

I have passed the proxy url at the top in the updated code, and implemented /search as an endpoint for the proxy because it gave much more than just coordinates, it also reflected the source URI, geo URI, OSM supported URI. I have changed this endpoint to /coordinates for now and have updated the proxy also.

I have passed the proxy url at the top in the updated code, and implemented /search as an endpoint for the proxy because it gave much more than just coordinates, it also reflected the source URI, geo URI, OSM supported URI. I have changed this endpoint to /coordinates for now and have updated the proxy also.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-24 23:58:39 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-24 23:58:39 +00:00

All the URIs for now return coordinates via the proxy, there are some of them that don't have a specific address but all of them return coordinates. I will also add address as a parameter in JSON response of /coordinates.

All the URIs for now return coordinates via the proxy, there are some of them that don't have a specific address but all of them return coordinates. I will also add address as a parameter in JSON response of /coordinates.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-25 00:03:48 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-25 00:03:48 +00:00

Implemented the translation for "ok" via localisation in the code, I have checked for different languages and it works. Reference for Marathi :-
OK_localisedString

Implemented the translation for "ok" via localisation in the code, I have checked for different languages and it works. Reference for Marathi :- <img width="340" alt="OK_localisedString" src="https://github.com/organicmaps/organicmaps/assets/120778728/c18582d9-abe3-4b3b-938e-6674ef3739ab">
kartikaysaxena (Migrated from github.com) reviewed 2023-06-25 00:06:33 +00:00
iphone/Maps/Classes/MapsAppDelegate.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-25 00:06:33 +00:00

It does only check the pasteboard once when the app becomes active, I have changed the name to clipboardChecker to avoid any misunderstanding. Also it is called under applicationDidBecomeActive() of MapsAppDelegate.mm

It does only check the pasteboard once when the app becomes active, I have changed the name to clipboardChecker to avoid any misunderstanding. Also it is called under applicationDidBecomeActive() of MapsAppDelegate.mm
biodranik (Migrated from github.com) requested changes 2023-06-25 20:25:53 +00:00
iphone/Maps/Classes/ClipboardMonitor.h
biodranik (Migrated from github.com) commented 2023-06-25 20:16:52 +00:00

check or extract or some better name

check or extract or some better name
iphone/Maps/Classes/ClipboardMonitor.mm
biodranik (Migrated from github.com) commented 2023-06-25 20:18:33 +00:00
  1. Includes are not sorted.
  2. Are they all needed?
1. Includes are not sorted. 2. Are they all needed?
biodranik (Migrated from github.com) commented 2023-06-25 20:19:39 +00:00 
NSString * privacyProxy = @"https://url-un.kartikay-2101ce32.workers.dev/coordinates?url=%@";

Please check our existing code style and fix it where necessary.

```suggestion NSString * privacyProxy = @"https://url-un.kartikay-2101ce32.workers.dev/coordinates?url=%@"; ``` Please check our existing code style and fix it where necessary.
biodranik (Migrated from github.com) commented 2023-06-25 20:20:27 +00:00

What if someone loses something important in the clipboard when opening OM?

What if someone loses something important in the clipboard when opening OM?
biodranik (Migrated from github.com) commented 2023-06-25 20:20:52 +00:00

Please check and fix formatting everywhere.

Please check and fix formatting everywhere.
biodranik (Migrated from github.com) commented 2023-06-25 20:22:30 +00:00

This code should be in the (better named) isURL

This code should be in the (better named) isURL
biodranik (Migrated from github.com) commented 2023-06-25 20:23:09 +00:00

We use two spaces for indentation. We don't use bracers for one-liners.

We use two spaces for indentation. We don't use bracers for one-liners.
biodranik (Migrated from github.com) commented 2023-06-25 20:24:04 +00:00
  1. How users should guess what "redirection failed" means?
  2. Why is it not localized properly?
1. How users should guess what "redirection failed" means? 2. Why is it not localized properly?
iphone/Maps/Classes/MapsAppDelegate.mm
biodranik (Migrated from github.com) commented 2023-06-25 20:25:17 +00:00

If the code runs in one function, can we avoid creating class at all, and define one function that internally may call a few other private functions?

If the code runs in one function, can we avoid creating class at all, and define one function that internally may call a few other private functions?
iphone/Maps/Maps.xcodeproj/project.pbxproj
biodranik (Migrated from github.com) commented 2023-06-25 20:25:46 +00:00

Why is this change needed?

Why is this change needed?
kartikaysaxena (Migrated from github.com) reviewed 2023-06-26 17:58:27 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-26 17:58:27 +00:00

Edited the text. Also there is no localized string (translations) present for redirection failed or something similar. There is one "Unable to locate the destination." , should I implement this?

Edited the text. Also there is no localized string (translations) present for redirection failed or something similar. There is one "Unable to locate the destination." , should I implement this?
kartikaysaxena (Migrated from github.com) reviewed 2023-06-26 18:00:09 +00:00
iphone/Maps/Classes/MapsAppDelegate.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-26 18:00:08 +00:00

I thought that by creating a separate class it would be better organised. I can change that if you want.

I thought that by creating a separate class it would be better organised. I can change that if you want.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-26 18:01:58 +00:00
iphone/Maps/Maps.xcodeproj/project.pbxproj
kartikaysaxena (Migrated from github.com) commented 2023-06-26 18:01:57 +00:00

This file is auto generated and edited by xcode,It specifies the file and class destinaton, I didn't include this file in an earlier commit, and then the build checks were failing.

This file is auto generated and edited by xcode,It specifies the file and class destinaton, I didn't include this file in an earlier commit, and then the build checks were failing.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-26 18:02:53 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-26 18:02:53 +00:00

I have changed that in the updated code, the clipboard is only cleared if it is a google maps url.

I have changed that in the updated code, the clipboard is only cleared if it is a google maps url.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-26 18:03:12 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-26 18:03:11 +00:00

Removed unnecessary imports.

Removed unnecessary imports.
kartikaysaxena (Migrated from github.com) reviewed 2023-06-26 18:06:53 +00:00
iphone/Maps/Classes/ClipboardMonitor.mm
kartikaysaxena (Migrated from github.com) commented 2023-06-26 18:06:52 +00:00

I have formatted the code.

I have formatted the code.
rtsisyk reviewed 2023-07-12 09:52:22 +00:00
android/src/app/organicmaps/MwmActivity.java
rtsisyk commented 2023-07-12 09:52:21 +00:00
Owner

Please don't reformat the file...

Please don't reformat the file...
kartikaysaxena (Migrated from github.com) reviewed 2023-07-12 19:29:25 +00:00
android/src/app/organicmaps/MwmActivity.java
kartikaysaxena (Migrated from github.com) commented 2023-07-12 19:29:25 +00:00

Reverted to original format.

Reverted to original format.
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
biodranik
Labels
Clear labels   
Accessibility

issues affecting users with specific needs

  
Accessibility

issues affecting users with specific needs

  
Address

Issues with addresses, post codes, city/town/village borders

  
Address

Issues with addresses, post codes, city/town/village borders

  
Android

Android development

  
Android

Android development

  
Android Auto

Android Auto

  
Android Auto

Android Auto

  
Android Automotive (AAOS)

   
Android Automotive (AAOS)

   
API

Short Links, deep links and url schemes, including ge0 and om://

  
API

Short Links, deep links and url schemes, including ge0 and om://

  
AppGallery

Huawei AppGallery

  
AppGallery

Huawei AppGallery

  
AppStore

Apple AppStore

  
AppStore

Apple AppStore

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Battery and Performance

Performance and battery consumption-related issues and optimizations

  
Blocker

An issue blocking the release

  
Blocker

An issue blocking the release

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Bookmarks and Tracks

Bookmarks, imported tracks, and KML, KMZ, KMB, GPX, GPZ import or export

  
Borders

Country borders and polygons

  
Borders

Country borders and polygons

  
Bug

Something isn't working

  
Bug

Something isn't working

  
Build

Compilation issue

  
Build

Compilation issue

  
CarPlay

CarPlay Integration

  
CarPlay

CarPlay Integration

  
Classificator

Classify object by types

  
Classificator

Classify object by types

  
Community

Community relations

  
Community

Community relations

  
Core

Cross-platform C++ libraries with a core functionality

  
Core

Cross-platform C++ libraries with a core functionality

  
CrashReports

Crash Reporting

  
CrashReports

Crash Reporting

  
Cycling

Bike, cycle, biking and related

  
Cycling

Bike, cycle, biking and related

  
Desktop

Desktop application

  
Desktop

Desktop application

  
DevEx

Developer Experience

  
DevEx

Developer Experience

  
DevOps

Automation and pipelines

  
DevOps

Automation and pipelines

  
dev_sandbox

A desktop app to debug rendering

  
dev_sandbox

A desktop app to debug rendering

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Directions

Issues with next turns, labels, streets, voice instructions

  
Documentation

Improvements or additions to documentation

  
Documentation

Improvements or additions to documentation

  
Downloader

Map dowloader

  
Downloader

Map dowloader

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Drape

Drape OpenGL, Vulkan and Metal graphics rendering engine

  
Driving

Drive a car related issues

  
Driving

Drive a car related issues

  
Duplicate

This issue or pull request already exists

  
Duplicate

This issue or pull request already exists

  
Editor

OSM Editor

  
Editor

OSM Editor

  
Elevation

Isolines and altitude

  
Elevation

Isolines and altitude

  
Enhancement

New feature or request, an improvement of some existing feature

  
Enhancement

New feature or request, an improvement of some existing feature

  
Epic

A large body work

  
Epic

A large body work

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
External Map Datasets

TIGER, Ordnance Survey, ASTER, SRTM, etc.

  
F-Droid

F-Droid build and distribution

  
F-Droid

F-Droid build and distribution

  
Fonts

Font and text rendering

  
Fonts

Font and text rendering

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Frequently User Reported

Most annoying UX and other issues for users reported by many people

  
Fund

   
Fund

   
Generator

Related to generator tool.

  
Generator

Related to generator tool.

  
Good first issue

Good for newcomers

  
Good first issue

Good for newcomers

  
Google Play

Google Play

  
Google Play

Google Play

  
GPS

Location and positioning issues

  
GPS

Location and positioning issues

  
GSoC

Ideas for Google Summer of Code

  
GSoC

Ideas for Google Summer of Code

  
iCloud

   
iCloud

   
Icons

Map and app icons

  
Icons

Map and app icons

  
iOS

iOS development

  
iOS

iOS development

  
Legal

Legal aspects

  
Legal

Legal aspects

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux Desktop

Ubuntu, Gentoo, Red Hat, Debian, Cent OS, Fedora, etc.

  
Linux packaging

   
Linux packaging

   
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Linux Phone

LibreM, PinePhone, Wayland and other mobile Linux versions

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Mac OS

Desktop issues related to Mac OS X, OM running on M1, M2, M3 Apple silicon, or Qt version

  
Map Data

OpenStreetMap data related issues

  
Map Data

OpenStreetMap data related issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Metro

Subway, Tube, light-rail public transport issues

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Navigation

Issues related to the active Navigator / Navigation mode

  
Need Feedback

Further information is requested

  
Need Feedback

Further information is requested

  
Night Mode

Night or dark mode feature

  
Night Mode

Night or dark mode feature

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
NLnet 2024-06-281

Tasks related to NLnet 2024-06-281 project.

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
No Feature Parity

A platform needs this to be implemented to have feature parity with other platforms

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Opening Hours

Business Hours, off time, holidays, seasonal

  
Outdoors

Hiking, mtb, 4x4...

  
Outdoors

Hiking, mtb, 4x4...

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
POI Info

Feature metadata, OSM tags that are displayed in Place Page

  
Privacy

Privacy

  
Privacy

Privacy

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Public Transport

Bus, trolleybus, train, tram, metro, subway, light rail

  
Raw Idea

An idea that needs to be elaborated

  
Raw Idea

An idea that needs to be elaborated

  
Refactoring

   
Refactoring

   
Regional

This issue is different depending on the country or region

  
Regional

This issue is different depending on the country or region

  
Regression

A regression bug

  
Regression

A regression bug

  
Releases

Meta-tickets for tracking the release procedure

  
Releases

Meta-tickets for tracking the release procedure

  
RoboTest

Robo / Monkey Auto Tests

  
RoboTest

Robo / Monkey Auto Tests

  
Route Planning

Preview and plan your track

  
Route Planning

Preview and plan your track

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Routing

Route building issues, e.g. valid route, valid ETA

  
Ruler

Helicopter routing, "as a crow flies"

  
Ruler

Helicopter routing, "as a crow flies"

  
Search

Search

  
Search

Search

  
Security

Security improvements

  
Security

Security improvements

  
Styles

Map drawing styles

  
Styles

Map drawing styles

  
Tests

   
Tests

   
Track Recording

Track Recording

  
Track Recording

Track Recording

  
Translations

Localization and translations issues

  
Translations

Localization and translations issues

  
TTS

Text-to-Speech

  
TTS

Text-to-Speech

  
UI

User interface issues

  
UI

User interface issues

  
UX

User eXperience, an issue with usability

  
UX

User eXperience, an issue with usability

  
Walk Navigation

Foot pedestrian mode

  
Walk Navigation

Foot pedestrian mode

  
Watches

Android Watches or Apple Watches support

  
Watches

Android Watches or Apple Watches support

  
Web

https://organicmaps.app/

  
Web

https://organicmaps.app/

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Wikipedia

Issues related to Wiki articles embedded in OM data files.

  
Windows

Windows

  
Windows

Windows

  
Won't fix

Unfeasible or impossible to resolve

  
Won't fix

Unfeasible or impossible to resolve

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

  
World Map

Issues about improving the base map of the whole world that is bundled with the app

No labels
Accessibility
Accessibility
Address
Address
Android
Android
Android Auto
Android Auto
Android Automotive (AAOS)
Android Automotive (AAOS)
API
API
AppGallery
AppGallery
AppStore
AppStore
Battery and Performance
Battery and Performance
Blocker
Blocker
Bookmarks and Tracks
Bookmarks and Tracks
Borders
Borders
Bug
Bug
Build
Build
CarPlay
CarPlay
Classificator
Classificator
Community
Community
Core
Core
CrashReports
CrashReports
Cycling
Cycling
Desktop
Desktop
DevEx
DevEx
DevOps
DevOps
dev_sandbox
dev_sandbox
Directions
Directions
Documentation
Documentation
Downloader
Downloader
Drape
Drape
Driving
Driving
Duplicate
Duplicate
Editor
Editor
Elevation
Elevation
Enhancement
Enhancement
Epic
Epic
External Map Datasets
External Map Datasets
F-Droid
F-Droid
Fonts
Fonts
Frequently User Reported
Frequently User Reported
Fund
Fund
Generator
Generator
Good first issue
Good first issue
Google Play
Google Play
GPS
GPS
GSoC
GSoC
iCloud
iCloud
Icons
Icons
iOS
iOS
Legal
Legal
Linux Desktop
Linux Desktop
Linux packaging
Linux packaging
Linux Phone
Linux Phone
Mac OS
Mac OS
Map Data
Map Data
Metro
Metro
Navigation
Navigation
Need Feedback
Need Feedback
Night Mode
Night Mode
NLnet 2024-06-281
NLnet 2024-06-281
No Feature Parity
No Feature Parity
Opening Hours
Opening Hours
Outdoors
Outdoors
POI Info
POI Info
Privacy
Privacy
Public Transport
Public Transport
Raw Idea
Raw Idea
Refactoring
Refactoring
Regional
Regional
Regression
Regression
Releases
Releases
RoboTest
RoboTest
Route Planning
Route Planning
Routing
Routing
Ruler
Ruler
Search
Search
Security
Security
Styles
Styles
Tests
Tests
Track Recording
Track Recording
Translations
Translations
TTS
TTS
UI
UI
UX
UX
Walk Navigation
Walk Navigation
Watches
Watches
Web
Web
Wikipedia
Wikipedia
Windows
Windows
Won't fix
Won't fix
World Map
World Map
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: organicmaps/organicmaps-tmp#5386
No description provided.
Delete branch "kartikaysaxena/master"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?