The other ones are missing several components. Here are the text versions of the formulas:
- Nefrit: Ca2(Mg,Fe)5Si8O22(OH)2
- Nephrite: Ca2 5Si8O22(OH)2
- Olivín: (Mg,Fe)2[SiO4]
- Olivine: 2SiO4
@Vachovec1 Thanks. These are even better test cases. We also need to remove the following attributes: rel, href, title. I'm going to explore running the html-sanitizer step before (in addition to after) the parentheses stripping since that whitelists attributes we want to keep instead of a blacklist of attributes we want to remove.
I'm running the compare script. How urgent is this to get deployed?
We don't want to be too aggressive when removing attributes since we don't want to remove needed attributes for other elements, like img, maybe figure and more.
Ugh, this is going to get ugly. The reason why (CH<sub id="mwCw">2</sub>) is stripped is because it does include a space -- in the HTML, before the id attribute. I can strip the id attributes beforehand but this doesn't guarantee that there wouldn't be other attributes leading to a strip of the parenthetical.
@Jdlrobson Sure. Will look into it. Thanks for the test case.
The MCS version does expose the revision for summaries but RESTBase does not. Not sure why. If I had to guess it may have to do with storage overhead. From what I remember is that there wasn't a use case for it.
@Jdlrobson f you don't want to wait you can purge the page with ?action=purge.
Yes, this one is still at 1.2.0.
@Jdlrobson I think MCS could still chose to drop any span.mw-redirects from summary output, knowing that these came from a.mw-redirects. Since the summary changes the links to spans.
Same on Safari on macOS.
@ssastry To see the difference you may need to look closely. I've increased the font size so it's easier to see. This is with Chrome. Note the difference in spacing where the diacritics are:
So, for reading purposes we should be able to get rid of them and flatten them completely.
Just explaining what's happening here technically:
Links with redirects get a special class in Parsoid: <a class="mw-redirect">.
When MCS creates the summary it turns the <a> tags into <span> tags,
so the result is: <span class="mw-redirect">. MCS removes the id attributes but class and style attributes would be preserved.
@ovasileva I found this on en:VPT: https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Hovercards. I haven't found any others yet.
The cswiki page summaries have been re-rendered with the correct thumbnail size. We're still working through other wikis but for cs the blank thumbnail image issue should be resolved.
@Jdlrobson Yep. The change was that the new summary was requesting a larger thumbnail image (for reasons unbeknownst to me), which resulted in the original image URL being returned for the thumbnail field as well. That one doesn't have the px- piece in the filename.
We should change both the server and the client side.
- On the server side, MCS should request smaller thumbnails for the summaries. The MCS patch is up ^ and will be deployed shortly. Once that is deployed @Pchelolo is going to run a dump of summaries over enwiki and cswiki (hopefully in parallel).
- The client should remove the assumption of having px- in the thumbnail URL.
Wed, Feb 21
Yes, it's in rest.js.
Correct. I can repro it on FF showing blank space. What I showed above was with Chrome.
E.g., when hovering over Timeline_of_the_far_future. The summary has the thumbnail
https://upload.wikimedia.org/wikipedia/commons/a/aa/Red_Giant_Earth_warm.jpg, which would have worked.
Instead it tries to download https://upload.wikimedia.org/wikipedia/commons/a/aa/320px-d_Giant_Earth_warm.jpg, which gets a 404.
Note the s/Red/320px-d/.
I can also reproduce the same on a few enwiki pages, see new subtask T187955.
The thumbnail in this page preview looks definitely wrong:
From https://cs.wikipedia.org/wiki/Donald_Trump hover over "prezident Spojených států amerických"
I suspect that this is a page preview (web client) issue. The /page/summary JSON responses for these pages look fine to me. All have the thumbnail property:
The summaries for all pages on cswiki are now rendered using MCS.
I'd like to hear from client teams what they think. If would be great to hear from devs familiar with the creation of the action=mobileview API.
Looks like the backlog of summary rerenders is worked off now, and I can see pages from the Recent Changes list getting the new MCS summary treatment automatically.
Tue, Feb 20
I guess we're in an unplanned, super-slow-rollout mode right now. While it works for manual purges, new pages showing up in recent changes don't get updated to 1.3.0 yet.
Here's the latest comparison run for the Czech html extracts from TextExtracts vs the MCS summary implementation: http://wpsummary.surge.sh/1.2.0-b0be98c/html/cs.html. See also T179875#3986429.
I've ran another compare run between the old 1.2.0 (TextExtracts) version and the latest (MCS repo b0be98c).
Sat, Feb 17
That's coming from the Parsoid response: https://en.wikipedia.org/api/rest_v1/page/html/Public_choice
Fri, Feb 16
Thu, Feb 15
Adding to that, I think for now the Android app should treat sections with negative section ids as uneditable, and wouldn't even show the pencil icon for these. On this article, editing the "India" section in the Android app works but none of its subheadings.
@Dbrant The -1 or -2 can theoretically happen on any page. Practically I have only observed that on non-main namespaces (talk or project are other namespaces that come to mind), though. This comes from Parsoid. IIUC those section ids mean that the section is not editable, mainly due to some template transclusion of a page with some HTML heading tags happening. If you want we could make a patch in MCS to change the values to something else. The problem is figuring out what that something else should be.
@cooltey RESTBase automatically resolves redirects for you. So that shouldn't be a problem.
Updated title and description of this task to reflect the current plan.
Wed, Feb 14
Update: since the main guys for this in the Services team are out today and tomorrow the earliest would be Tuesday, Feb 20.
Mon, Feb 12
Just to be explicit, the issue was probably not the non-200 error code but the empty payload.
Sounds good. I think we should be able to get that one in by today's deployment window if we all agree on what value we should use for the type field in the cases we were returning 204 before. I brought up none as a strawman proposal. Is that ok or should we use a different value?
Thanks! How about we change the spec to return 200s and empty extracts in the cases we were returning 204s?
Also let the apps still prepare for 204s in the future.
Sun, Feb 11
Maybe the interaction of page previews and link previews is different enough to warrant more thought/discussion about the behavior differences. There's a delay before page previews appear on web but none before link previews in the Android app because if Link Previews are enabled that's currently the only way to follow most links. We don't want a delay here because that would slow down navigation in the Android app. (The iOS app is still another story since one has to hard press the link to get the link preview there.)
Sat, Feb 10
I think this is probably due to T186927 which happened today. Looks like some of the summary results need to be purged/re-rendered. I guess the ones that had a rerender triggered during the time the bug existed and until the fix was deployed. Not sure if it's worth forcing a rerender on all summaries if we still go ahead with the switchover to summary 1.3.0 soon. We were planning to do that on Monday it might be delayed, see T186933.
Well, it's really up to @Fjalapeno and @phuedx to hash out if we do go ahead on Monday or later. One possible option would be to change the spec to return 200 but a special type for when not to show previews. That should be BWC for the apps. Or we allow more time for the apps to patch things. Or we say that this only affects a few pages and is rare enough that we can go ahead with this on Monday. (Although the Android crash is bad, of course.)
@Dbrant I think we want to not show a link preview in this case instead of the error I'm seeing with this patch:
@Dbrant Nice! Maybe there is hope for the current spec after all. When do you think this could be released? Could this be back-ported to older Android versions which don't get the updates?
Fri, Feb 9
Due to T186933 we should postpone the summary switchover until a proper solution has been agreed and deployed.
Hmm, for which Glossary word (on what page) does it work? It doesn't work for "mantis"->"mantisees"-> "plural", as @cooltey mentioned earlier.
Thanks, the Kibana dashboard for MCS over the last 12 hours looks much cleaner already. :)
Sorry, we found another issue, see new subtask, which delayed the switchover to Monday morning.
Thu, Feb 8
Are there any older app versions out there which might use a different method to get the data? Does Android use MCS?
@Mholloway What is it blocked on?
Wed, Feb 7
@Tgr The iOS app does it directly through action=mobileview but that should be easy enough to change there to page_props as well if necessary. The harder part would be changing the description editing workflow on the apps.
Question: is this only for English WP?
@Jdlrobson MCS is currently using action=mobileview for the main pages since the content of the main page content is not responsive. Hoping for T138622#3416133 to get done. Then we could get rid of the special handling there.
Some comments between @Mholloway and myself on IRC -services channel earlier with minor edits: