erlyaws / yaws

Yaws webserver
https://erlyaws.github.io
BSD 3-Clause "New" or "Revised" License
1.28k stars 267 forks source link

another exhtml bug #217

Open evbogdanov opened 9 years ago

evbogdanov commented 9 years ago
Internal error, yaws code crashed

ERROR erlang code threw an uncaught exception:
 File: /Users/ev/erlang/apps/yawstest/www/exhtml.yaws:2
Class: error
Exception: {case_clause,{inline,yes,<<"p">>}}
Req: {http_request,'GET',{abs_path,"/ab.yaws"},{1,1}}
Stack: [{yaws_exhtml,format,5,[{file,"src/yaws_exhtml.erl"},{line,82}]},
        {yaws_exhtml,format,5,[{file,"src/yaws_exhtml.erl"},{line,117}]},
        {yaws_exhtml,format,5,[{file,"src/yaws_exhtml.erl"},{line,119}]},
        {yaws_exhtml,format,5,[{file,"src/yaws_exhtml.erl"},{line,117}]},
        {yaws_exhtml,format,2,[{file,"src/yaws_exhtml.erl"},{line,66}]},
        {yaws_server,handle_out_reply,5,
                     [{file,"src/yaws_server.erl"},{line,3179}]},
        {yaws_server,deliver_dyn_part,8,
                     [{file,"src/yaws_server.erl"},{line,2817}]},
        {yaws_server,aloop,4,[{file,"src/yaws_server.erl"},{line,1230}]}]

How to reproduce:

out(_Arg) ->
    Struct = {html, [], [
        {head, [], [
            {meta, [{charset, <<"utf-8">>}]},
            {title, [], <<"title">>}
        ]},
        {body, [], [
            {p, [], <<"p">>}
        ]}
    ]},
    {exhtml, Struct}.

As before, ehtml version is ok (see issue #216)

vinoski commented 9 years ago

Thanks, I'll have a look at this as well.

vinoski commented 9 years ago

I'm wondering if there might be more exhtml problems lurking beyond this one, so I pushed branch exhtml-fixes that fixes this problem. @evbogdanov, can you try it and let me know if you run into any other exhtml bugs? If you see any other issues I can then fix them on that branch as well. Thanks.

evbogdanov commented 9 years ago

@vinoski, nice! I'll check it out tomorrow.

I want to ask. Have you considered merging ehtml / exhtml into one-to-rule-them-all format?

The point is just to make this beast compatible with HTML5. After all, this is what newcomers expect from such feature.

vinoski commented 9 years ago

@evbogdanov that's a good question for @klacke.

evbogdanov commented 9 years ago

@vinoski, I did some testing: https://github.com/evbogdanov/yaws_exhtml_fixes

Overall, I think the branch is usable. But a couple of bugs were found.

IMO, the number one bug is an inconsistency between ehtml / exhtml. For example:

<!-- ehtml -->
<ul>
<li>foo</li>
<li>bar</li>
<li>baz</li></ul>

vs

<!-- exhtml -->
<ul>
  <li>
    foo
  </li>
  <li>
    bar
  </li>
  <li>
    baz
  </li>
</ul>

Another example:

EXHTML = {h1, [], {lists, max, [[1, 2, 3]]}},
{exhtml, EXHTML}.
% Internal error, yaws code crashed...

EHTML = {h1, [], {lists, max, [[1, 2, 3]]}},
{ehtml, EHTML}.
% no error?

And one more:

EXHTML = {h1, [], 123},
{exhtml, EXHTML}.
% <h1>
%   123
% </h1>

EHTML = {h1, [], 123},
{ehtml, EHTML}.
% <h1>{</h1>

EXHTML = {h1, [], 1234},
{exhtml, EXHTML}.
% <h1>
%   1234
% </h1>

EHTML = {h1, [], 1234},
{ehtml, EHTML}.
% Internal error, yaws code crashed...
vinoski commented 9 years ago

There are bound to be differences between ehtml and exhtml, as they're intended to be different. I wasn't involved in putting the exhtml support together but I would guess it's meant to be more strict and to follow the XHTML spec, where ehtml was just more about being able to realize HTML using Erlang terms and it's not intended to be restrictive. Again, @klacke is probably the best person to describe the origins and intentions of each.

That said, some of what you're showing with your examples seems questionable, so I'll look into those issues a bit.

klacke commented 9 years ago

If I remember correctly, at the time there was no XHTML, there was only lax HTML. Maybe there was (XHTML) but is wasn't on the radar at the time. The exhtml tag was added later.

evbogdanov commented 9 years ago

@klacke @vinoski thanks for your replies!

Guys, have a look at ehtml5.

It's my HTML5 rethinking of ehtml with a couple enhancements: 1) Doctype is auto generated by {html} struct. 2) Tag attributes use maps instead of proplists. 3) Ability to use structs like {Tag, Content}. In ehtml it looks like {Tag, [], Content}, and I constantly forget adding these empty lists :-)

Personally, I think ehtml / ehtml5 can become very interesting with further improvements. But what do you think? Should we spend more time on this or it's ok how it's now?

vinoski commented 9 years ago

@evbogdanov I haven't looked at it in detail, but one initial comment is that the use of maps is problematic since not everyone using Yaws uses a version of Erlang/OTP supporting maps.