* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-11 18:04 [PATCH 0/5] Convert to HTML, fix HTML validation issues wub
@ 2016-05-11 18:04 ` wub
2016-05-11 18:56 ` john
` (2 more replies)
2016-05-11 18:04 ` [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links" wub
` (3 subsequent siblings)
4 siblings, 3 replies; 35+ messages in thread
From: wub @ 2016-05-11 18:04 UTC (permalink / raw)
Get rid of the XHTML headers, bringing cgit slowly to the modern age of
HTML.
---
ui-shared.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/ui-shared.c b/ui-shared.c
index 9a38aa9..1ded2d6 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -12,8 +12,7 @@
#include "html.h"
static const char cgit_doctype[] =
-"<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"\n"
-" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">\n";
+"<!DOCTYPE html>"\n";
static char *http_date(time_t t)
{
@@ -723,7 +722,7 @@ void cgit_print_docstart(void)
char *host = cgit_hosturl();
html(cgit_doctype);
- html("<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>\n");
+ html("<html lang='en'>\n");
html("<head>\n");
html("<title>");
html_txt(ctx.page.title);
--
2.8.1
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-11 18:04 ` [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html> wub
@ 2016-05-11 18:56 ` john
2016-05-11 19:28 ` wub
2016-05-12 15:31 ` Jason
2016-05-12 15:32 ` Jason
2016-05-12 15:38 ` Jason
2 siblings, 2 replies; 35+ messages in thread
From: john @ 2016-05-11 18:56 UTC (permalink / raw)
On Wed, May 11, 2016 at 06:04:14PM +0000, Juuso Lapinlampi wrote:
> Get rid of the XHTML headers, bringing cgit slowly to the modern age of
> HTML.
This seems like a reasonable aim, but don't we need to actually *be*
HTML(5?) as well in order to do this? Currently we close <input/> and
<img/> tags even though HTML is explicit that this shouldn't be done.
I think we need to fix all of those as well if we're going to claim to
be HTML rather than XHTML. I suspect that would be a bit big to review
as a single patch, but I certainly think that it should be applied as a
single patch series.
> ---
> ui-shared.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/ui-shared.c b/ui-shared.c
> index 9a38aa9..1ded2d6 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -12,8 +12,7 @@
> #include "html.h"
>
> static const char cgit_doctype[] =
> -"<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"\n"
> -" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">\n";
> +"<!DOCTYPE html>"\n";
>
> static char *http_date(time_t t)
> {
> @@ -723,7 +722,7 @@ void cgit_print_docstart(void)
>
> char *host = cgit_hosturl();
> html(cgit_doctype);
> - html("<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>\n");
> + html("<html lang='en'>\n");
> html("<head>\n");
> html("<title>");
> html_txt(ctx.page.title);
> --
> 2.8.1
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-11 18:56 ` john
@ 2016-05-11 19:28 ` wub
2016-05-12 15:31 ` Jason
1 sibling, 0 replies; 35+ messages in thread
From: wub @ 2016-05-11 19:28 UTC (permalink / raw)
On Wed, May 11, 2016 at 07:56:19PM +0100, John Keeping wrote:
> I think we need to fix all of those as well if we're going to claim to
> be HTML rather than XHTML. I suspect that would be a bit big to review
> as a single patch, but I certainly think that it should be applied as a
> single patch series.
It shouldn't be a squashed "single patch", but rather merged with
`git merge --no-ff` to keep the full history.
But I agree, this is still too incomplete to be merged.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-11 18:56 ` john
2016-05-11 19:28 ` wub
@ 2016-05-12 15:31 ` Jason
1 sibling, 0 replies; 35+ messages in thread
From: Jason @ 2016-05-12 15:31 UTC (permalink / raw)
On Wed, May 11, 2016 at 8:56 PM, John Keeping <john at keeping.me.uk> wrote:
> This seems like a reasonable aim, but don't we need to actually *be*
> HTML(5?) as well in order to do this? Currently we close <input/> and
> <img/> tags even though HTML is explicit that this shouldn't be done.
No, in HTML5 you can do it if you want but you don't have to.
http://blog.teamtreehouse.com/to-close-or-not-to-close-tags-in-html5
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-11 18:04 ` [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html> wub
2016-05-11 18:56 ` john
@ 2016-05-12 15:32 ` Jason
2016-05-12 18:58 ` wub
2016-05-12 15:38 ` Jason
2 siblings, 1 reply; 35+ messages in thread
From: Jason @ 2016-05-12 15:32 UTC (permalink / raw)
Merged!
It's time we grow up.
If this causes rendering errors due to compatibility modes the prior
XHTML header was triggering, we can fix those up in subsequent
commits.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-12 15:32 ` Jason
@ 2016-05-12 18:58 ` wub
2016-05-12 19:04 ` Jason
0 siblings, 1 reply; 35+ messages in thread
From: wub @ 2016-05-12 18:58 UTC (permalink / raw)
On Thu, May 12, 2016 at 05:32:32PM +0200, Jason A. Donenfeld wrote:
> Merged!
>
> It's time we grow up.
>
> If this causes rendering errors due to compatibility modes the prior
> XHTML header was triggering, we can fix those up in subsequent
> commits.
Note that I will be unable to work on fixing the potential errors, at
least for few days. If anyone has free time to work on this, go ahead.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html>
2016-05-11 18:04 ` [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html> wub
2016-05-11 18:56 ` john
2016-05-12 15:32 ` Jason
@ 2016-05-12 15:38 ` Jason
2 siblings, 0 replies; 35+ messages in thread
From: Jason @ 2016-05-12 15:38 UTC (permalink / raw)
On Wed, May 11, 2016 at 8:04 PM, Juuso Lapinlampi <wub at partyvan.eu> wrote:
>
> static const char cgit_doctype[] =
> -"<!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.0 Transitional//EN\"\n"
> -" \"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd\">\n";
> +"<!DOCTYPE html>"\n";
Not syntactically valid C either. Test your commits before you submit.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-11 18:04 [PATCH 0/5] Convert to HTML, fix HTML validation issues wub
2016-05-11 18:04 ` [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html> wub
@ 2016-05-11 18:04 ` wub
2016-05-11 18:44 ` john
2016-05-11 18:04 ` [PATCH 3/5] Revert "ui-shared: add rel-vcs microformat links to HTML header" wub
` (2 subsequent siblings)
4 siblings, 1 reply; 35+ messages in thread
From: wub @ 2016-05-11 18:04 UTC (permalink / raw)
vcs-git is not a registered keyword. This microformat does not seem to
be very popular and will throw errors in HTML validators.
Because the <link> element leaves us with no sensible rel attribute to
use, there's no reason to keep this.
For future consideration, microdata should be marked with Schema.org
markup instead.
This reverts commit d31be4ccc2f978edd2a40c2721e1efdc1eee2343.
---
ui-summary.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/ui-summary.c b/ui-summary.c
index 8e81ac4..375bd45 100644
--- a/ui-summary.c
+++ b/ui-summary.c
@@ -31,11 +31,9 @@ static void print_url(const char *url)
htmlf("<tr class='nohover'><th class='left' colspan='%d'>Clone</th></tr>\n", columns);
}
- htmlf("<tr><td colspan='%d'><a rel='vcs-git' href='", columns);
+ htmlf("<tr><td colspan='%d'><a href='", columns);
html_url_path(url);
- html("' title='");
- html_attr(ctx.repo->name);
- html(" Git repository'>");
+ html("'>");
html_txt(url);
html("</a></td></tr>\n");
}
--
2.8.1
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-11 18:04 ` [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links" wub
@ 2016-05-11 18:44 ` john
2016-05-12 15:34 ` Jason
2016-05-12 16:49 ` pabs3
0 siblings, 2 replies; 35+ messages in thread
From: john @ 2016-05-11 18:44 UTC (permalink / raw)
On Wed, May 11, 2016 at 06:04:15PM +0000, Juuso Lapinlampi wrote:
> vcs-git is not a registered keyword. This microformat does not seem to
> be very popular and will throw errors in HTML validators.
>
> Because the <link> element leaves us with no sensible rel attribute to
> use, there's no reason to keep this.
>
> For future consideration, microdata should be marked with Schema.org
> markup instead.
>
> This reverts commit d31be4ccc2f978edd2a40c2721e1efdc1eee2343.
I'm negative on this, since we had a request to add support [1] and I
generally treat validator errors like compiler warnings: some of them
aren't very useful and should be disabled.
Also the whatwg HTML spec [2] says:
Extensions to the predefined set of link types may be registered
in the microformats wiki existing-rel-values page. [MFREL]
Now, admittedly, vcs-git isn't registered there but I'm also not
convinced HTML validators will be checking there, and the spec goes on
to say:
Anyone is free to edit the microformats wiki existing-rel-values
page at any time to add a type.
So I'm not convinced that this really is non-conformant.
[1] https://lists.zx2c4.com/pipermail/cgit/2014-August/002191.html
[2] https://html.spec.whatwg.org/multipage/semantics.html#other-link-types
> ---
> ui-summary.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/ui-summary.c b/ui-summary.c
> index 8e81ac4..375bd45 100644
> --- a/ui-summary.c
> +++ b/ui-summary.c
> @@ -31,11 +31,9 @@ static void print_url(const char *url)
> htmlf("<tr class='nohover'><th class='left' colspan='%d'>Clone</th></tr>\n", columns);
> }
>
> - htmlf("<tr><td colspan='%d'><a rel='vcs-git' href='", columns);
> + htmlf("<tr><td colspan='%d'><a href='", columns);
> html_url_path(url);
> - html("' title='");
> - html_attr(ctx.repo->name);
> - html(" Git repository'>");
> + html("'>");
> html_txt(url);
> html("</a></td></tr>\n");
> }
> --
> 2.8.1
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-11 18:44 ` john
@ 2016-05-12 15:34 ` Jason
2016-05-12 15:37 ` pabs3
2016-05-12 19:09 ` wub
2016-05-12 16:49 ` pabs3
1 sibling, 2 replies; 35+ messages in thread
From: Jason @ 2016-05-12 15:34 UTC (permalink / raw)
Hi Paul,
We added vcs-git per your request [1]. Now there's talk of removing it.
Could you give full justification for its existence? How is it useful?
What uses it?
Thanks,
Jason
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-12 15:34 ` Jason
@ 2016-05-12 15:37 ` pabs3
2016-05-12 15:47 ` Jason
2016-05-12 19:09 ` wub
1 sibling, 1 reply; 35+ messages in thread
From: pabs3 @ 2016-05-12 15:37 UTC (permalink / raw)
On Thu, 2016-05-12 at 17:34 +0200, Jason A. Donenfeld wrote:
> We added vcs-git per your request [1]. Now there's talk of removing it.
What are the reasons for removing it?
> Could you give full justification for its existence? How is it useful?
> What uses it?
The spec should answer all these questions:
https://joeyh.name/rfc/rel-vcs/
--
bye,
pabs
http://bonedaddy.net/pabs3/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160512/9ba1f9d9/attachment.asc>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-12 15:34 ` Jason
2016-05-12 15:37 ` pabs3
@ 2016-05-12 19:09 ` wub
2016-05-13 0:26 ` pabs3
1 sibling, 1 reply; 35+ messages in thread
From: wub @ 2016-05-12 19:09 UTC (permalink / raw)
I am okay with keeping the vcs-git, but I don't have very high hopes for
it before validator.nu recognizes it is a valid rel attribute.
Seeing as how WHATWG is pushing Schema.org language in itemprops ahead,
I suggest vcs-git to be replaced with Schema.org, e.g.
SoftwareSourceCode.[1]
On Thu, May 12, 2016 at 05:34:50PM +0200, Jason A. Donenfeld wrote:
> Could you give full justification for its existence? How is it useful?
> What uses it?
Machine parsing. Google uses Schema.org markup in HTML to enhance Google
Search results (e.g. IMDB).
[1]: https://schema.org/SoftwareSourceCode
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-12 19:09 ` wub
@ 2016-05-13 0:26 ` pabs3
0 siblings, 0 replies; 35+ messages in thread
From: pabs3 @ 2016-05-13 0:26 UTC (permalink / raw)
On Thu, 2016-05-12 at 19:09 +0000, Juuso Lapinlampi wrote:
> I am okay with keeping the vcs-git, but I don't have very high hopes for
> it before validator.nu recognizes it is a valid rel attribute.
What a validator does or does not do is pretty irrelevant.
> Seeing as how WHATWG is pushing Schema.org language in itemprops ahead,
> I suggest vcs-git to be replaced with Schema.org, e.g.
> SoftwareSourceCode.[1]
SoftwareSourceCode looks useless for the purpose of vcs-* since there
is no qualification of what kind of repository is present.
--
bye,
pabs
http://bonedaddy.net/pabs3/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160513/a8279b38/attachment-0001.asc>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links"
2016-05-11 18:44 ` john
2016-05-12 15:34 ` Jason
@ 2016-05-12 16:49 ` pabs3
1 sibling, 0 replies; 35+ messages in thread
From: pabs3 @ 2016-05-12 16:49 UTC (permalink / raw)
John Keeping wrote:
> we had a request to add support [1]?
I was the one who requested it.
> Now, admittedly, vcs-git isn't registered there
...
> Anyone is free to edit the microformats wiki existing-rel-values
> page at any time to add a type.
I've now registered vcs-* on that page.
> So I'm not convinced that this really is non-conformant.
Agreed.
The initial blog post about this and the spec itself give the
rationale, uses, producers and consumers of this format. Admittedly it
isn't supported by much software but reducing that isn't the way to go.
https://joeyh.name/blog/entry/proposing_rel-vcs/
https://joeyh.name/rfc/rel-vcs/
--
bye,
pabs
http://bonedaddy.net/pabs3/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160513/c4e35d70/attachment.asc>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 3/5] Revert "ui-shared: add rel-vcs microformat links to HTML header"
2016-05-11 18:04 [PATCH 0/5] Convert to HTML, fix HTML validation issues wub
2016-05-11 18:04 ` [PATCH 1/5] ui-shared: HTML-ize DOCTYPE and <html> wub
2016-05-11 18:04 ` [PATCH 2/5] Revert "ui-summary: add "rel='vcs-git'" to clone URL links" wub
@ 2016-05-11 18:04 ` wub
2016-05-11 18:45 ` john
2016-05-11 18:04 ` [PATCH 4/5] ui: Fix bad value for attribute action on form elements wub
2016-05-11 18:04 ` [PATCH 5/5] ui-shared: Remove a name attribute with an empty value wub
4 siblings, 1 reply; 35+ messages in thread
From: wub @ 2016-05-11 18:04 UTC (permalink / raw)
See: a24995bf8c55114044d6baf32ad5e8c3a04d924d
This reverts commit 3c53ebfb57a5dba8fc65b2f99ebbfb6356666e34.
---
ui-shared.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/ui-shared.c b/ui-shared.c
index 1ded2d6..ba11c55 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -703,15 +703,6 @@ void cgit_redirect(const char *url, bool permanent)
html("\n\n");
}
-static void print_rel_vcs_link(const char *url)
-{
- html("<link rel='vcs-git' href='");
- html_attr(url);
- html("' title='");
- html_attr(ctx.repo->name);
- html(" Git repository'/>\n");
-}
-
void cgit_print_docstart(void)
{
if (ctx.cfg.embedded) {
@@ -753,8 +744,6 @@ void cgit_print_docstart(void)
strbuf_release(&sb);
free(fileurl);
}
- if (ctx.repo)
- cgit_add_clone_urls(print_rel_vcs_link);
if (ctx.cfg.head_include)
html_include(ctx.cfg.head_include);
html("</head>\n");
--
2.8.1
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 3/5] Revert "ui-shared: add rel-vcs microformat links to HTML header"
2016-05-11 18:04 ` [PATCH 3/5] Revert "ui-shared: add rel-vcs microformat links to HTML header" wub
@ 2016-05-11 18:45 ` john
2016-05-11 20:28 ` wub
0 siblings, 1 reply; 35+ messages in thread
From: john @ 2016-05-11 18:45 UTC (permalink / raw)
On Wed, May 11, 2016 at 06:04:16PM +0000, Juuso Lapinlampi wrote:
> See: a24995bf8c55114044d6baf32ad5e8c3a04d924d
You can't do this, the commit hash will be different when the patch is
applied from an email.
> This reverts commit 3c53ebfb57a5dba8fc65b2f99ebbfb6356666e34.
Same comments apply as for 2/5.
> ---
> ui-shared.c | 11 -----------
> 1 file changed, 11 deletions(-)
>
> diff --git a/ui-shared.c b/ui-shared.c
> index 1ded2d6..ba11c55 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -703,15 +703,6 @@ void cgit_redirect(const char *url, bool permanent)
> html("\n\n");
> }
>
> -static void print_rel_vcs_link(const char *url)
> -{
> - html("<link rel='vcs-git' href='");
> - html_attr(url);
> - html("' title='");
> - html_attr(ctx.repo->name);
> - html(" Git repository'/>\n");
> -}
> -
> void cgit_print_docstart(void)
> {
> if (ctx.cfg.embedded) {
> @@ -753,8 +744,6 @@ void cgit_print_docstart(void)
> strbuf_release(&sb);
> free(fileurl);
> }
> - if (ctx.repo)
> - cgit_add_clone_urls(print_rel_vcs_link);
> if (ctx.cfg.head_include)
> html_include(ctx.cfg.head_include);
> html("</head>\n");
> --
> 2.8.1
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 3/5] Revert "ui-shared: add rel-vcs microformat links to HTML header"
2016-05-11 18:45 ` john
@ 2016-05-11 20:28 ` wub
0 siblings, 0 replies; 35+ messages in thread
From: wub @ 2016-05-11 20:28 UTC (permalink / raw)
On Wed, May 11, 2016 at 07:45:34PM +0100, John Keeping wrote:
> On Wed, May 11, 2016 at 06:04:16PM +0000, Juuso Lapinlampi wrote:
> > See: a24995bf8c55114044d6baf32ad5e8c3a04d924d
>
> You can't do this, the commit hash will be different when the patch is
> applied from an email.
Oops. Sometimes I forget git-send-email(1) doesn't equal to
git-format-patch(1) patch files, which I often share on IRC with
different projects.
See: https://notabug.org/WubTheCaptain/cgit/commit/b0f9e4edbd6ec2682332b77b1c4e52a1e7d5b12c.
The branch is wub/modernize-html.
This is also one big reason why merges matter and are preferred over
squashed commits, as you claimed privately Jason to not normally merge
via branches (which I consider bad practice).
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-11 18:04 [PATCH 0/5] Convert to HTML, fix HTML validation issues wub
` (2 preceding siblings ...)
2016-05-11 18:04 ` [PATCH 3/5] Revert "ui-shared: add rel-vcs microformat links to HTML header" wub
@ 2016-05-11 18:04 ` wub
2016-05-11 18:50 ` john
2016-05-12 15:41 ` Jason
2016-05-11 18:04 ` [PATCH 5/5] ui-shared: Remove a name attribute with an empty value wub
4 siblings, 2 replies; 35+ messages in thread
From: wub @ 2016-05-11 18:04 UTC (permalink / raw)
The action attribute must "a valid non-empty URL potentially surrounded
by spaces."
See: https://html.spec.whatwg.org/#the-form-element
---
ui-shared.c | 2 +-
ui-stats.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui-shared.c b/ui-shared.c
index 9a38aa9..0b7fdec 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -940,7 +940,7 @@ static void print_header(void)
cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
if (ctx.env.authenticated) {
html("</td><td class='form'>");
- html("<form method='get' action=''>\n");
+ html("<form method='get' action='.'>\n");
cgit_add_hidden_formfields(0, 1, ctx.qry.page);
html("<select name='h' onchange='this.form.submit();'>\n");
for_each_branch_ref(print_branch_option, ctx.qry.head);
diff --git a/ui-stats.c b/ui-stats.c
index 8cd9178..1fa634f 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -389,7 +389,7 @@ void cgit_show_stats(void)
cgit_print_layout_start();
html("<div class='cgit-panel'>");
html("<b>stat options</b>");
- html("<form method='get' action=''>");
+ html("<form method='get' action='.'>");
cgit_add_hidden_formfields(1, 0, "stats");
html("<table><tr><td colspan='2'/></tr>");
if (ctx.repo->max_stats > 1) {
--
2.8.1
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-11 18:04 ` [PATCH 4/5] ui: Fix bad value for attribute action on form elements wub
@ 2016-05-11 18:50 ` john
2016-05-12 15:41 ` Jason
1 sibling, 0 replies; 35+ messages in thread
From: john @ 2016-05-11 18:50 UTC (permalink / raw)
On Wed, May 11, 2016 at 06:04:17PM +0000, Juuso Lapinlampi wrote:
> The action attribute must "a valid non-empty URL potentially surrounded
> by spaces."
>
> See: https://html.spec.whatwg.org/#the-form-element
Makes sense (although missing sign-off again):
Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
> ui-shared.c | 2 +-
> ui-stats.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui-shared.c b/ui-shared.c
> index 9a38aa9..0b7fdec 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -940,7 +940,7 @@ static void print_header(void)
> cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
> if (ctx.env.authenticated) {
> html("</td><td class='form'>");
> - html("<form method='get' action=''>\n");
> + html("<form method='get' action='.'>\n");
> cgit_add_hidden_formfields(0, 1, ctx.qry.page);
> html("<select name='h' onchange='this.form.submit();'>\n");
> for_each_branch_ref(print_branch_option, ctx.qry.head);
> diff --git a/ui-stats.c b/ui-stats.c
> index 8cd9178..1fa634f 100644
> --- a/ui-stats.c
> +++ b/ui-stats.c
> @@ -389,7 +389,7 @@ void cgit_show_stats(void)
> cgit_print_layout_start();
> html("<div class='cgit-panel'>");
> html("<b>stat options</b>");
> - html("<form method='get' action=''>");
> + html("<form method='get' action='.'>");
> cgit_add_hidden_formfields(1, 0, "stats");
> html("<table><tr><td colspan='2'/></tr>");
> if (ctx.repo->max_stats > 1) {
> --
> 2.8.1
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-11 18:04 ` [PATCH 4/5] ui: Fix bad value for attribute action on form elements wub
2016-05-11 18:50 ` john
@ 2016-05-12 15:41 ` Jason
2016-05-12 19:22 ` wub
1 sibling, 1 reply; 35+ messages in thread
From: Jason @ 2016-05-12 15:41 UTC (permalink / raw)
Could we instead just remove action? Does this work both in the case
of path/to/page and path/to/page/ ? Or in the former will this wind up
requesting path/to/.?
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-12 15:41 ` Jason
@ 2016-05-12 19:22 ` wub
2016-05-12 19:27 ` Jason
0 siblings, 1 reply; 35+ messages in thread
From: wub @ 2016-05-12 19:22 UTC (permalink / raw)
On Thu, May 12, 2016 at 05:41:50PM +0200, Jason A. Donenfeld wrote:
> Could we instead just remove action? Does this work both in the case
> of path/to/page and path/to/page/ ? Or in the former will this wind up
> requesting path/to/.?
I don't understand your question very well. I am pretty sure the action
attribute is required in form. And yes, it works both ways.
'.' means "this page", and that's what the UI code uses in many other
places. The piece of code itself is used around branch switching next to
the "switch button" near top of the document.
I tested this in Chromium "developer" tools and the '.' should work
without issues to keep the page the user was on.
So in the end, you get directed to path/to/page or path/to/page/
depending on which you had. This just fixes a validation issue.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-12 19:22 ` wub
@ 2016-05-12 19:27 ` Jason
2016-05-12 19:29 ` Jason
2016-05-12 19:36 ` wub
0 siblings, 2 replies; 35+ messages in thread
From: Jason @ 2016-05-12 19:27 UTC (permalink / raw)
Good idea with the Chrome developer tools. I just tried it myself, and
no it doesn't work.
The switch branch form works with https://git.zx2c4.com/zmusic-ng/ and
fails with https://git.zx2c4.com/zmusic-ng which is what I suspected.
It turns out action is not required either, and when omitted means
"this page", which is what we want and what I originally suggested:
https://www.w3.org/TR/html5/forms.html#attr-fs-action
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-12 19:27 ` Jason
@ 2016-05-12 19:29 ` Jason
2016-05-12 19:36 ` wub
1 sibling, 0 replies; 35+ messages in thread
From: Jason @ 2016-05-12 19:29 UTC (permalink / raw)
https://git.zx2c4.com/cgit/commit/?id=c34e28835bc06ea9f76f440909f59a697910e9e8
Voila.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 4/5] ui: Fix bad value for attribute action on form elements
2016-05-12 19:27 ` Jason
2016-05-12 19:29 ` Jason
@ 2016-05-12 19:36 ` wub
2016-05-12 19:39 ` Jason
1 sibling, 1 reply; 35+ messages in thread
From: wub @ 2016-05-12 19:36 UTC (permalink / raw)
On Thu, May 12, 2016 at 09:27:14PM +0200, Jason A. Donenfeld wrote:
> Good idea with the Chrome developer tools. I just tried it myself, and
> no it doesn't work.
>
> The switch branch form works with https://git.zx2c4.com/zmusic-ng/ and
> fails with https://git.zx2c4.com/zmusic-ng which is what I suspected.
>
> It turns out action is not required either, and when omitted means
> "this page", which is what we want and what I originally suggested:
>
> https://www.w3.org/TR/html5/forms.html#attr-fs-action
You're right, it does fail. Thanks for testing.
The "diff options" in commit diff page also uses `action='.'`. That also
creates an issue.
The (more) relevant WHATWG specification about action is there:
https://html.spec.whatwg.org/multipage/forms.html#attr-fs-action
So the action is optional.
Would this patch work to fix the validation and related bugs?
diff --git a/ui-diff.c b/ui-diff.c
index 52ed942..edee793 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -340,7 +340,7 @@ void cgit_print_diff_ctrls(void)
html("<div class='cgit-panel'>");
html("<b>diff options</b>");
- html("<form method='get' action='.'>");
+ html("<form method='get'>");
cgit_add_hidden_formfields(1, 0, ctx.qry.page);
html("<table>");
html("<tr><td colspan='2'/></tr>");
diff --git a/ui-shared.c b/ui-shared.c
index 770b685..2c88b72 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -937,7 +937,7 @@ static void print_header(void)
cgit_summary_link(ctx.repo->name, ctx.repo->name, NULL, NULL);
if (ctx.env.authenticated) {
html("</td><td class='form'>");
- html("<form method='get' action=''>\n");
+ html("<form method='get'>\n");
cgit_add_hidden_formfields(0, 1, ctx.qry.page);
html("<select name='h' onchange='this.form.submit();'>\n");
for_each_branch_ref(print_branch_option, ctx.qry.head);
diff --git a/ui-stats.c b/ui-stats.c
index 8cd9178..7acd358 100644
--- a/ui-stats.c
+++ b/ui-stats.c
@@ -389,7 +389,7 @@ void cgit_show_stats(void)
cgit_print_layout_start();
html("<div class='cgit-panel'>");
html("<b>stat options</b>");
- html("<form method='get' action=''>");
+ html("<form method='get'>");
cgit_add_hidden_formfields(1, 0, "stats");
html("<table><tr><td colspan='2'/></tr>");
if (ctx.repo->max_stats > 1) {
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 5/5] ui-shared: Remove a name attribute with an empty value
2016-05-11 18:04 [PATCH 0/5] Convert to HTML, fix HTML validation issues wub
` (3 preceding siblings ...)
2016-05-11 18:04 ` [PATCH 4/5] ui: Fix bad value for attribute action on form elements wub
@ 2016-05-11 18:04 ` wub
2016-05-11 18:52 ` john
2016-05-12 15:43 ` Jason
4 siblings, 2 replies; 35+ messages in thread
From: wub @ 2016-05-11 18:04 UTC (permalink / raw)
The name attribute is optional in an input element, but it must not be
an empty value.
See: https://html.spec.whatwg.org/#attr-fe-name
See: https://html.spec.whatwg.org/#the-input-element
---
ui-shared.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ui-shared.c b/ui-shared.c
index 0b7fdec..a0dec5e 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -947,7 +947,7 @@ static void print_header(void)
if (ctx.repo->enable_remote_branches)
for_each_remote_ref(print_branch_option, ctx.qry.head);
html("</select> ");
- html("<input type='submit' name='' value='switch'/>");
+ html("<input type='submit' value='switch'/>");
html("</form>");
}
} else
--
2.8.1
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 5/5] ui-shared: Remove a name attribute with an empty value
2016-05-11 18:04 ` [PATCH 5/5] ui-shared: Remove a name attribute with an empty value wub
@ 2016-05-11 18:52 ` john
2016-05-12 15:43 ` Jason
1 sibling, 0 replies; 35+ messages in thread
From: john @ 2016-05-11 18:52 UTC (permalink / raw)
On Wed, May 11, 2016 at 06:04:18PM +0000, Juuso Lapinlampi wrote:
> The name attribute is optional in an input element, but it must not be
> an empty value.
>
> See: https://html.spec.whatwg.org/#attr-fe-name
> See: https://html.spec.whatwg.org/#the-input-element
Again, sign-off is needed, but otherwise:
Reviewed-by: John Keeping <john at keeping.me.uk>
> ---
> ui-shared.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui-shared.c b/ui-shared.c
> index 0b7fdec..a0dec5e 100644
> --- a/ui-shared.c
> +++ b/ui-shared.c
> @@ -947,7 +947,7 @@ static void print_header(void)
> if (ctx.repo->enable_remote_branches)
> for_each_remote_ref(print_branch_option, ctx.qry.head);
> html("</select> ");
> - html("<input type='submit' name='' value='switch'/>");
> + html("<input type='submit' value='switch'/>");
> html("</form>");
> }
> } else
> --
> 2.8.1
>
> _______________________________________________
> CGit mailing list
> CGit at lists.zx2c4.com
> http://lists.zx2c4.com/mailman/listinfo/cgit
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH 5/5] ui-shared: Remove a name attribute with an empty value
2016-05-11 18:04 ` [PATCH 5/5] ui-shared: Remove a name attribute with an empty value wub
2016-05-11 18:52 ` john
@ 2016-05-12 15:43 ` Jason
1 sibling, 0 replies; 35+ messages in thread
From: Jason @ 2016-05-12 15:43 UTC (permalink / raw)
Merged, thanks.
^ permalink raw reply [flat|nested] 35+ messages in thread