You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by EdisonXu <gi...@git.apache.org> on 2014/07/29 10:05:47 UTC
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
GitHub user EdisonXu opened a pull request:
https://github.com/apache/incubator-storm/pull/209
Set the encoding of Storm UI to UTF-8
storm UI imports the jquery-tablesorter project, in which there're codes as below:
```
return /^[£$€?.]/.test(s);
```
original source code:
https://github.com/christianbach/tablesorter/blob/master/jquery.tablesorter.js?#L910
These special characters will bring problems for some encoding, e.g GBK.
The topology summary will not display normally.
Better force the encoding to UTF-8.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/EdisonXu/incubator-storm master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-storm/pull/209.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #209
----
commit 6639abb6f9664038a41d9dfdf1f5934dbdbc4384
Author: Edison <ed...@outlook.com>
Date: 2014-07-29T07:51:29Z
Add UTF-8 encoding
commit b1a64443943b554365a47f6053c51789da5f87f3
Author: Edison <ed...@outlook.com>
Date: 2014-07-29T07:51:48Z
Add UTF-8 encoding
commit 98fa84f66c6c41d8e23088bb5bc96ef88e27b67f
Author: Edison <ed...@outlook.com>
Date: 2014-07-29T07:52:05Z
Add UTF-8 encoding
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-52250001
I am +1.
@EdisonXu, Could you file a [JIRA](https://issues.apache.org/jira/browse/STORM) for this request, and add \[STORM-X\] (for the new number) to the title of this pull request?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54247768
@EdisonXu I'm +1 too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-53493304
Created [STORM-475](https://issues.apache.org/jira/browse/STORM-475) to track this pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54245880
@HeartSaVioR , oh. I didn't aware this change. OK then. It'll solve this prob indeed.
However, I still suggest make STORM UI use UTF-8 encoding by default.
What's your opinion?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54566774
@d2r Thanks for merging. My JIRA username is xeseo.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54242896
Hi @d2r , I just tried to move the <code>meta</code> to be the 1st child of <code>head</code> tag, and I didn't find any prob.
What's the problem you met? Could you pls describe it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54244513
@EdisonXu We changed tablesorter.js's master from christianbach to Mottie.
original source code (current master) is https://github.com/Mottie/tablesorter/blob/master/js/jquery.tablesorter.js
It already replaces currency to unicode representation, so I said there seems to no problem.
```
ts.addParser({
id: "currency",
is: function(s) {
return (/^\(?\d+[\u00a3$\u20ac\u00a4\u00a5\u00a2?.]|[\u00a3$\u20ac\u00a4\u00a5\u00a2?.]\d+\)?$/).test((s || '').replace(/[+\-,. ]/g,'')); // £$€¤¥¢
},
format: function(s, table) {
var n = ts.formatFloat((s || '').replace(/[^\w,. \-()]/g, ""), table);
return s && typeof n === 'number' ? n : s ? $.trim( s && table.config.ignoreCase ? s.toLocaleLowerCase() : s ) : s;
},
type: "numeric"
});
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54327829
> Hi @d2r , I just tried to move the meta to be the 1st child of head tag, and I didn't find any prob.
> What's the problem you met? Could you pls describe it?
If I open the Firefox developer console, there is a warning (not an error) about the meta tag appearing after byte 1024 and therefore causing the browser to reload the page.
> @HeartSaVioR , oh. I didn't aware this change. OK then. It'll solve this prob indeed.
> However, I still suggest make STORM UI use UTF-8 encoding by default.
> What's your opinion?
Still I think this is a good idea. I tested this earlier and while UTF-8 is handled by the tablesorter rendering, anything outside of tablesorter is not UTF-8. So I think it would be better to just make the encoding for the page UTF-8.
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54244935
@EdisonXu same part of jquery.tablesorter.min.js at current master
```
n.addParser({id:"currency",is:function(d){return/^\(?\d+[\u00a3$\u20ac\u00a4\u00a5\u00a2?.]|[\u00a3$\u20ac\u00a4\u00a5\u00a2?.]\d+\)?$/.test((d|| "").replace(/[+\-,. ]/g,""))},format:function(d,v){var p=n.formatFloat((d||"").replace(/[^\w,. \-()]/g,""),v);return d&&"number"===typeof p?p:d?g.trim(d&&v.config.ignoreCase?d.toLocaleLowerCase():d):d},type:"numeric"});
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-storm/pull/209
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54475217
@EdisonXu Yes, it is fixed now. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-52263376
Actually your issue is fixed by #174, but PR may help for other modification.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/209#discussion_r16745733
--- Diff: storm-core/src/ui/public/component.html ---
@@ -27,6 +27,7 @@
<script src="/js/bootstrap-twipsy.js" type="text/javascript"></script>
<script src="/js/script.js" type="text/javascript"></script>
<script src="/js/moment.min.js" type="text/javascript"></script>
+<meta charset="UTF-8">
--- End diff --
We need to move this `meta` tag into the first 1024 bytes of the document so that some browsers (FF) do not need to reload the page in order to get the encoding correct.
I suggest we move these tags up to be the first child of `head` tag.
Likewise with the other changes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-52558324
+1 too, @d2r if you want to you can file a JIRA for this issue, and check it in without fully linking the two.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54242144
sorry ,didn't see the feedback. will check asap
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54388635
Hi @d2r , I just downloaded a latest firefox and installed, and I didn't see the warning you mentioned in the console. Could you pls try it again with Ctrl+F5 to refresh the cache? Maybe it's caused by setting the meta as the last child. After putting it at the first place, I didn't see this warning again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54248529
OK. I've update the commit to make <code>meta</code> to be the 1st child of <code>head</code>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by EdisonXu <gi...@git.apache.org>.
Github user EdisonXu commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54242782
@HeartSaVioR, I believe this is different problem from #174.
The regular expression used here is still there even in latest tablesorter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54355882
I am still +1.
@d2r Thanks for the very complete review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-54477412
@EdisonXu Thanks, I merged this to master. If you want attribution for STORM-475 in JIRA, let me know what JIRA user name is yours.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: Set the encoding of Storm UI to UTF-...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/209#issuecomment-53500001
Hi @EdisonXu, I just noticed that putting `meta` after the first 1024th byte of the page can cause problems. Could you take a look?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---