Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

morebits: Use new error formats (default to html) in api; use $.parseHTML in status messages #1179

Merged

Conversation

Amorymeltzer
Copy link
Collaborator

2nd commit uses/enforces the newer API error messages in Morebits.wiki.api, defaulting to html, but allowing plaintext and wikitext; raw, none, and the legacy bc will be converted to html. Provides errorlang: mw.config.get('wgUserLanguage'). Errors are limited to the first found, which was the case with the old syntax and is assumed by our uses of errorCode/errorText, but maybe something to refactor at some point (not a huge gain IMO).

The 1st commit enables the html showing up nicely, by using $.parseHTML in Morebits.status.codify(). There's a whole long commit message, but the tl;dr is: it will now automatically parse html, not wikitext; and there's now a statRaw for error logging.

@Amorymeltzer Amorymeltzer added Feature request Other wikis non-EnWiki issues and language/i18n/l10n stuff Module: morebits The morebits.js library deprecations Functions, etc. from Morebits that are deprecated and should be checked for usage before removal labels Oct 31, 2020
@Amorymeltzer Amorymeltzer changed the title morebits: Use new error formats (default to html) in mb.w.api; us $.parseHTML in status messages morebits: Use new error formats (default to html) in api; use $.parseHTML in status messages Oct 31, 2020
@Amorymeltzer
Copy link
Collaborator Author

Presumably there are places that could be simplified by making use of the 1st commit, but I've not done a survey and might as well leave well-enough alone atm. But block and protect for sure.

@siddharthvp
Copy link
Member

Well, this was part of the plan in #1144, though I was more of expecting to rewrite Morebits.status with fancy StatusLine and StatusBlock classes.

I would bat for uselang: 'content', errorlang: 'content' (same effect as specifying mw.config.get('wgContentLanguage') as everything in Twinkle is in the content language, and errorsuselocal: true.

@siddharthvp
Copy link
Member

One more note: because morebits.css has
form.quickform * { font-family: sans-serif; }
any status elements that would be expected to affect font-family won't work properly. In particular, things like <pre>, <kbd>, and <code> won't display with font-family: monospace, as may be desired/expected, until/unless morebits.css is changed.

That CSS should be removed. sans-serif is I think the default font on mediawiki, so there's no need to specify it. Even if it were different for someone, I don't see why it needs to be adjusted away from that choice.

@Amorymeltzer
Copy link
Collaborator Author

I would bat for uselang: 'content', errorlang: 'content' (same effect as specifying mw.config.get('wgContentLanguage') as everything in Twinkle is in the content language, and errorsuselocal: true.

I can never keep track of these! But, this is for messages being passed to the Twinkle user, not ever being added to the wiki. The content in the Twinkle interface is... well, it's all en now, but in theory it should show the user 'user' and edit/act with 'content', no? user is the default for uselang, but errorlang needs to be set explicitly (the default to match uselang was... weirdly nonfunctional for me?).

That CSS should be removed. sans-serif is I think the default font on mediawiki, so there's no need to specify it. Even if it were different for someone, I don't see why it needs to be adjusted away from that choice.

Jah, but figured OoS for this. IIRC from minor testing, it was basically just the select menus that would/could be affected: without this, they'd inherit from the skin, so Modern/Timeless would look different (slightly bigger). Don't think it's a big deal, but didn't look too much into it.

@siddharthvp
Copy link
Member

but in theory it should show the user 'user' and edit/act with 'content', no?

In theory yes, but in practise, that's hopelessly complicated to work out. There could be situations where one could want to have a tooltip say "for xxx purposes use {{other template name}} instead". Here the "other template name" obviously has to be based on the content language, but the message itself should be in user language? You see, using one language for the UI and another for editing is a lost cause. My i18n plans (assuming they approve the grant) have been to use content lang for everything.

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Nov 10, 2020
Defaults to `html`, but `plaintext` and `wikitext` are acceptable; `raw`, `none`, and the legacy format `bc` will be converted to `html`.  Currently limited to the first error found, which was the case with the old syntax and is assumed by our uses of `errorCode`/`errorText`.

Of note, this also defaults all queries to the wiki's `content` language, see discussion in wikimedia-gadgets#1179.

https://www.mediawiki.org/wiki/API:Errors_and_warnings
@Amorymeltzer
Copy link
Collaborator Author

That's a real good point — ugh! Done and done!

Amorymeltzer added a commit to Amorymeltzer/twinkle that referenced this pull request Nov 25, 2020
Defaults to `html`, but `plaintext` and `wikitext` are acceptable; `raw`, `none`, and the legacy format `bc` will be converted to `html`.  Currently limited to the first error found, which was the case with the old syntax and is assumed by our uses of `errorCode`/`errorText`.

Of note, this also defaults all queries to the wiki's `content` language, see discussion in wikimedia-gadgets#1179.

https://www.mediawiki.org/wiki/API:Errors_and_warnings
Inserting and rendering HTML was always possible in `Morebits.status`, since `codify` would simply append an Element if present, but while providing an HTMLElement would work as expected a text string of HTML would be treated as just that: a text string.  In theory, assigning each node's `innerHTML` would work, but (as structured) we'd have to do it separately, since `Morebits.status`' `text` (first parameter, before colon) and `stat` (second parameter, after colon) have their `document-fragment` HTML built/added separately, in `.generate` and `.render`, respectively.  Moreover, it would make for poor console warning/error messages, since we had `text` and `textRaw` but just `stat` (in fact, we've been using `status` instead of `this.stat` since 2013 (26c3add)).  Instead, this modifies `codify` to `$.parseHTML` any strings passed it and append the subsequent Element(s).  Should make rendering HTML in status messages simpler and more straightforward.

This could, of course, have all sorts of side effects, although I think the likelihood is rather small, given that people don't generally have a desire to show raw HTML text in an output.

Of note, however, is that, since we're using `$.parseHTML`, we're of course parsing *HTML* and not *wikitext*.  Parsing a wikitext link into an HTML link isn't too tricky, something like works:

```javascript
var link = document.createElement('a');
link.setAttribute('href', mw.util.getUrl(page));
link.appendChild(document.createTextNode(title));
```

Of course, that's assuming we have the wikilink we want, and not just a blob of wikitext.  We could use regex to pull things out, something like `/\[\[([^\]\|]+)(?:\|([^\|\]]+))?\]\]/`, and parse individually, but the more "proper" way is probably using `Morebits.unbinder` or something; something perhaps for `Morebits.wikitext.page` to learn.

One more note: because morebits.css has
```css
form.quickform *
{
	font-family: sans-serif;
}
```

any status elements that would be expected to affect `font-family` won't work properly.  In particular, things like `<pre>`, `<kbd>`, and `<code>` won't display with `font-family: monospace`, as may be desired/expected, until/unless morebits.css is changed.

Required adding `Element` to the repl testing.
Defaults to `html`, but `plaintext` and `wikitext` are acceptable; `raw`, `none`, and the legacy format `bc` will be converted to `html`.  Currently limited to the first error found, which was the case with the old syntax and is assumed by our uses of `errorCode`/`errorText`.

Of note, this also defaults all queries to the wiki's `content` language, see discussion in wikimedia-gadgets#1179.

https://www.mediawiki.org/wiki/API:Errors_and_warnings
@Amorymeltzer Amorymeltzer merged commit d1768d5 into wikimedia-gadgets:master Dec 5, 2020
@Amorymeltzer Amorymeltzer deleted the morebits-errorformat branch December 5, 2020 15:51
@Amorymeltzer Amorymeltzer added this to the December 2020 update milestone Dec 5, 2020
Amorymeltzer added a commit that referenced this pull request Dec 8, 2020
- `errorformat` defaults to `html`, only accepting `wikitext` and `plaintext` otherwise (#1179)
- `uselang` is set to the wiki's content language (#1179)
- `formatversion` defaults to `2` when `format=json` (#1038)
siddharthvp added a commit that referenced this pull request Oct 21, 2022
In #1179, support for new error formats were added and used by default. However, the error handling code still assumes the legacy errorformat, which passes details of the first `error`, while in the newer ones we have an array `errors`.
siddharthvp added a commit that referenced this pull request Oct 21, 2022
In #1179, support for new error formats were added and used by default. However, the error handling code still assumes the legacy errorformat, which passes details of the first `error`, while in the newer ones we have an array `errors`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecations Functions, etc. from Morebits that are deprecated and should be checked for usage before removal Feature request Module: morebits The morebits.js library Other wikis non-EnWiki issues and language/i18n/l10n stuff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants