r89222 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89221‎ | r89222 | r89223 >
Date:19:43, 31 May 2011
Author:preilly
Status:old (Comments)
Tags:
Comment:
remove unused format name
Modified paths:
  • /trunk/extensions/PatchOutputMobile/DeviceDetection.php (modified) (history)
  • /trunk/extensions/PatchOutputMobile/PatchOutputMobile.php (modified) (history)

Diff [purge]

Index: trunk/extensions/PatchOutputMobile/DeviceDetection.php
——@@ -360,7 +360,7 @@
361361
362362 if ( $formatName === '' ) {
363363 if ( strpos( $acceptHeader, 'application/vnd.wap.xhtml+xml' ) !== false ) {
364 - $formatName = 'wap2';
 364+ $formatName = 'wml';
365365 } elseif ( strpos( $acceptHeader, 'vnd.wap.wml' ) !== false ) {
366366 $formatName = 'wml';
367367 } else {
Index: trunk/extensions/PatchOutputMobile/PatchOutputMobile.php
——@@ -39,7 +39,7 @@
4040 'onOutputPageBeforeHTML' );
4141
4242 class ExtPatchOutputMobile {
43 - const VERSION = '0.4.1';
 43+ const VERSION = '0.4.2';
4444
4545 private $doc;
4646

Follow-up revisions

RevisionCommit summaryAuthorDate
r89992change default for vnd.wap.xhtmlpreilly18:42, 13 June 2011

Comments

#Comment by TheDJ (talk | contribs)   21:57, 12 June 2011

This is incorrect. XHTML Mobile Profile (aka WAP 2.0) is totally different from WML.

http://en.wikipedia.org/wiki/XHTML_Mobile_Profile

http://en.wikipedia.org/wiki/Wireless_Markup_Language

#Comment by TheDJ (talk | contribs)   23:49, 12 June 2011

Basically, you will want at least three basic markups I think.

In WURFL these would be:

html_web_4_0 (iphone android etc) wml_1_1/wml_1_2/wml_1_3 (traditional wap phones. 1.2 is the most widely deployed i believe, but I see little reason to support more than 1.1) xhtml_wi_oma_xhtmlmp_1_0 (Wap 2.0/ XHTML Mobile Profile) This section has huge per phone diversity in capabilities more specific targeting per device might be required.

preferred_markup contains the preferred type of a device in WURFL. Also some splitting in the html4 section might be desirable between markup for touch and for keypad devices.

#Comment by Preilly (talk | contribs)   02:56, 31 August 2011

We are sticking with two formats for now.

#Comment by TheDJ (talk | contribs)   06:29, 31 August 2011

limiting scope seems like a fine idea to me. This is simply wrong however. You are making the active assumption here that something that announces to support XHTML MP 1.0 will support WML. Since the two don't even look like eachother, there should at the very least be a very visible. "FIXME" in the source code.

#Comment by Preilly (talk | contribs)   17:19, 31 August 2011

This is not what the code currently looks like:

} elseif ( strpos( $acceptHeader, 'vnd.wap.wml' ) !== false ) { $formatName = 'wml';

#Comment by TheDJ (talk | contribs)   21:05, 31 August 2011

No, in the current code, we would then be talking about, as changed in r89992

if ( strpos( $acceptHeader, 'application/vnd.wap.xhtml+xml' ) !== false ) { $formatName = 'html';

which is still not correct of course.

#Comment by Preilly (talk | contribs)   21:12, 31 August 2011

How is that not correct?

#Comment by TheDJ (talk | contribs)   21:17, 31 August 2011

if ( strpos( $acceptHeader, 'text/html' ) !== false ) { $formatName = 'html'; is correct

and

if ( strpos( $acceptHeader, 'application/vnd.wap.xhtml+xml' ) !== false ) { $formatName = 'xhtml mp'; // wap2.0 whatever you want to name it.

xhtml mp is much more limited then html. If a device were to support html, it would report as text/html, not as wap 2.0.

#Comment by Preilly (talk | contribs)   21:18, 31 August 2011

Okay, but we don't have a xhtml mp view at this time that is why I'm selecting the html based view.

#Comment by TheDJ (talk | contribs)   21:21, 31 August 2011

Which is why I said that there should a be a big FIXME indicating the fact that we know that it is wrong, but that we have no alternative atm.

#Comment by Preilly (talk | contribs)   21:22, 31 August 2011

Makes sense. Thanks, for the clarification. As always, I appreciate your feedback.

Status & tagging log