wordpress-mobile / WordPress-iOS

WordPress for iOS - Official repository
http://ios.wordpress.org/
GNU General Public License v2.0
3.66k stars 1.11k forks source link

Reader: When possible links in a post detail should open in the Reader #4971

Open aerych opened 8 years ago

aerych commented 8 years ago

Whenever a tapped link in the post detail points to a wpcom blog post, open and display the post in the reader detail rather than the in-app browser.

Requested by @m

blowery commented 8 years ago

That's going to require API support I think. We'll need to parse the content and look for links that seem to be to WPCOM sites and add in some special data- attributes that tell the client what post to load.

aerych commented 8 years ago

It might be simpler than that if we focus just on links that match a "sitename.wordpress.com" pattern. Perhaps /sites/sitename.wordpress.com/posts/postID could be extended to accept the path + slug in lieu of postID

TooTallNate commented 8 years ago

That would catch the *.wordpress.com sites, but wouldn't catch custom domain names.

aerych commented 8 years ago

Yep. The thought being it would make a first pass simpler to implement. My fear is processing and modifying each link server-side will be tricky and/or computationally prohibitive.

But if we consider support for links to custom domains as critical, perhaps it would be just as easy to create a new endpoint to accept a url as its argument and let it return a matching post object.

blowery commented 8 years ago

But if we consider support for links to custom domains as critical, perhaps it would be just as easy to create a new endpoint to accept a url as its argument and let it return a matching post object.

That sounds like a lot of calls. :D How would that work? Parse the document for links and issue a batch to try and resolve all of them to wp posts?

blowery commented 8 years ago

Yep. The thought being it would make a first pass simpler to implement. My fear is processing and modifying each link server-side will be tricky and/or computationally prohibitive.

Yeah. It doesn't sound cheap. Amounts to a parser to find the links, then a spin over the host to determine if we host it, then a switch_to_blog and a url_to_post_id to gather the post id. Unsure how that would play for VIPs.

And we have to do this for all posts, even feeds, not just posts from sites we control. At least feeds we could do this work on ingest... Though that breaks if a site moves off wpcom down the road. Hrm. Almost have to do it as an output filter / client-side thing.

@gibrown any thoughts?

aerych commented 8 years ago

That sounds like a lot of calls. :D How would that work? Parse the document for links and issue a batch to try and resolve all of them to wp posts?

I'm thinking rather than trying to parse all the links server-side just the clients check on demand. Most links won't be clicked on so processing them all is a bit wasteful. Why not just let the client query as needed for a post object by passing the URL in question? If a post isn't returned the client can then just hand the link off to the in-app webview as normal. There is an avg cost of a few milliseconds but that's not terrible.

blowery commented 8 years ago

There is an avg cost of a few milliseconds but that's not terrible.

I think it's going to be a lot worse than that since you're depending on a cell network... It'll feel really laggy won't it? Touch / wait / .... / wait ... / hey a view!

aerych commented 8 years ago

The API call to check for a post should be pretty light weight and I think we can mitigate any perceived lag by querying for the web page the same time as querying for a matching post object. If there was a match it would feel pretty much the same as opening a post detail from a notification. If there was no match the webview should be well on its way to be already loaded.

nbradbury commented 8 years ago

I've been wanting this for as long as I can remember :)

I think @aerych is right that checking on demand is the right approach, despite the potential for lag.

However, rather than have an endpoint which returns a post object for a URL, we could get away with returning just the IDs necessary to request the post object. The detail views in both apps already have the smarts to request a post object when it's not cached locally.

That would make the call more lightweight, so the client could more quickly determine whether a link goes to a wp post. And if the call is more lightweight, it'd be more reasonable for the clients to issue requests for the first couple of links when the detail is opened.

gibrown commented 8 years ago

@blowery we already parse all links out for ES. :) Also pull out the host name separately. We don't currently do a lookup to see if the hosted content is on .com or Jetpack or whether we already have it somewhere, but the urls are also in ES.

So you could get the doc from ES, grab all the links, and then see you can find them in the ES index. If we index feeds then this method could work there too. Or we could adjust our indexing to pull out the blog_ids in with the links. The big hole in this is sometimes there are multiple urls pointing at any one article and we don't always have them. Or there could be params or hashes that we don't have indexed. But could probably make something work.

aerych commented 8 years ago

we could get away with returning just the IDs necessary to request the post object.

I considered this, but then that would be yet another request to make. Since the second request would just be to fetch the a post already known to exist it seemed more efficient to just return the post.

But now you've got me thinking, maybe there is still a better approach. Perhaps instead of querying the REST API when a link is clicked it would be better to query the API as the post details is opened passing a list of all links on that particular post. The API return a dictionary of matches (siteID/postID). Then the clients can check against the results if its best to open the link in the reader or a webview. One query per article opened vs one per link and we an cache the results client side if needed.

Thoughts?

blowery commented 8 years ago

Perhaps instead of querying the REST API when a link is clicked it would be better to query the API as the post details is opened passing a list of all links on that particular post. The API return a dictionary of matches (siteID/postID). Then the clients can check against the results if its best to open the link in the reader or a webview. One query per article opened vs one per link and we an cache the results client side if needed.

https://github.com/wordpress-mobile/WordPress-iOS/issues/4971#issuecomment-198375335

Sounds good? :D

aerych commented 8 years ago

Doh!! @blowery, did I misunderstand your earlier comment? I did didn't I? I thought it was still thinking of handing all the things server-side before sending anything to the client.
Obviously I needed more :coffee:

rachelmcr commented 7 years ago

I know the original issue is about links in the post detail, but when we implement this we should also try to capture links in comments and in notifications (e.g. comment notifications) and open them in the Reader detail instead of in-app browser.