bugzilla.dtd does not include any custom fields (automate DTD creation)

RESOLVED FIXED in Bugzilla 4.4

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: cso, Assigned: dkl)

Tracking

unspecified
Bugzilla 4.4
Bug Flags:
approval +

Details

(Whiteboard: [bmo4.0-resolved])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
Since bugzilla.dtd is a static document, it doesn't contain any of the custom field definitions that are defined within the XML.

We need to find some way to either generate bugzilla.dtd dynamically, or update it on creation and deletion of custom fields.

Comment 1

12 years ago
Updated summary to make it easier to find this bug.
Summary: bugzilla.dtd does not include any custom fields → bugzilla.dtd does not include any custom fields (automate DTD creation)

Comment 2

11 years ago
This looks like it's blocking or related to bug #346249. Since custom fields always fail to validate, users have to engineer their own (possibly malformed) DTD to validate and import bugs with.

Implementing this will keep the DTD up to date, make importing bugs to bugzilla simpler, and increase user base by reducing time/complexity to adopt bugzilla.

Updated

7 years ago
Duplicate of this bug: 346249

Updated

7 years ago
Depends on: 538428

Comment 4

7 years ago
mkanat, what's your preferred solution:

1) Regenerate bugzilla.dtd every time editfields.cgi edits a field.
2) Regenerate bugzilla.dtd every time we run checksetup.pl.
3) Create some bugzilla_dtd.cgi which generates bugzilla.dtd on the fly.

The problem with 1) is that a CGI script is not allowed to write in bugzilla/. This wouldn't be a problem if bugzilla.dtd was in bugzilla/data/ though.

The problem with 2) is that we would be out of sync most of the time as we don't run checksetup.pl very often in a production environment.

3) This could work if XML validators accept to point to a .cgi script instead of a .dtd file (I don't know what the spec says about it).
(Reporter)

Comment 5

7 years ago
DTDs do not have to be external documents, they can be 'in-line' within the XML file, so theoretically there's a couple more options:

4) Generate the DTD as part of xml.cgi, and include it in-line in the returned document.
5) Generate a static DTD file (as above) and then read the file contents and include it in the xml.cgi script output (so xml.cgi is not regenerating the DTD on each load)

The one caveat to them is that the DTD being in-line will increase the size of xml.cgi output.

Comment 6

7 years ago
(In reply to comment #5)
> The one caveat to them is that the DTD being in-line will increase the size of
> xml.cgi output.

Yeah, that's something we should avoid to do.
(Assignee)

Comment 7

7 years ago
It cannot be an auto-generated static DTD document as the custom fields in BMO (at least) are filtered based on product. So each bug would potentially need a different DTD file depending on which fields are visible or not. 

Putting the DTD data inline in the XML output is not entirely un-doable as it really doesn't add a whole lot of extra data to the XML size. Except for the custom fields the rest of the DTD doesn't change much at all. The bulk of the data size will be the bug's XML anyway especially if there are a lot of comments 
and/or attachments. 

Also adding it inline would be easier to maintain than having a separate .cgi and associated templates. And not to mention one less call to the server. 

dkl
Assignee: general → dkl
Status: NEW → ASSIGNED

Comment 8

7 years ago
(In reply to comment #7)
> It cannot be an auto-generated static DTD document as the custom fields in BMO
> (at least) are filtered based on product.

Each custom field could be marked as optional, not required. That's not really an issue.


> Putting the DTD data inline in the XML output is not entirely un-doable as it
> really doesn't add a whole lot of extra data to the XML size.

It does if you filter data being displayed in the XML file. See e.g. https://bugzilla.mozilla.org/show_bug.cgi?ctype=xml&id=341809&field=short_desc&field=bug_id

You would add a lot of extra data, and unless you plan to validate the XML file, you don't care about it.


> Also adding it inline would be easier to maintain than having a separate .cgi
> and associated templates.

That's one advantage, I agree.
(Assignee)

Comment 9

7 years ago
(In reply to comment #8)
> It does if you filter data being displayed in the XML file. See e.g.
> https://bugzilla.mozilla.org/show_bug.cgi?ctype=xml&id=341809&field=short_desc&field=bug_id
> 
> You would add a lot of extra data, and unless you plan to validate the XML
> file, you don't care about it.

Well if we wanted to get really complex we could make the DTD match the requested fields as well which would cut down drastically the size of the inline DTD.

But really the current DTD is only around 3k in size. We are not talking about alot. 

Will the XML validate if the DTD has more fields than the XML document? As long as they are not marked as REQUIRED?

dkl

Comment 10

7 years ago
(In reply to comment #9)
> Will the XML validate if the DTD has more fields than the XML document? As long
> as they are not marked as REQUIRED?

I don't see why not. That's the goal of #IMPLIED. But if we do this, all fields should then be marked as optional as you can override the list of fields to display via the "field" argument. That's not something we should do, IMO.

Comment 11

7 years ago
  Sorry it took me so long to reply, I just got this bugmail!

(In reply to comment #4)
> 3) Create some bugzilla_dtd.cgi which generates bugzilla.dtd on the fly.

  I think that's probably the best choice. Then we can template it, too.
(Assignee)

Comment 12

7 years ago
I still think it may be best to include the DTD in the XML output. If we make a separate bugzilla_dtd.cgi, it will use people's cookies to determine which fields are viewable or not (at least for BMO). But if the user is using a command line or offline utility to validate such as xmllint, it will try to use the URL in the XML document to validate and it will connect as an anonymous which may cause some fields to not be included.

I suppose that upstream could argue that they do not care that BMO has custom fields that are hidden based on permissions, but that is the reality of our situation.

dkl

Comment 13

7 years ago
(In reply to comment #12)
> I still think it may be best to include the DTD in the XML output.

  No, no, that wouldn't be good for XML-using clients. There are a ton of tools that grab the XML output numerous times and depend on small rapid responses. You really don't want to add 3K to every XML request if you can help it.

  Also, there are almost zero clients that require a DTD, so including it in every request would just be wasteful for the vast majority of cases. Making the DTD itself entirely dynamic is a much better solution, I think.

Comment 14

7 years ago
  But you know, if we made it a template, you could pass an option, &dtd=1, and get the DTD inside of the document. That would solve both problems, no? :-)
(Assignee)

Comment 15

7 years ago
Created attachment 537573 [details] [diff] [review]
Patch to move bugzilla.dtd to template (v1)

First crack at this for early review. Seems to work well in my testing and can be used as either cgi or included in the show.xml.tmpl template.

dkl
Attachment #537573 - Flags: review?(mkanat)

Comment 16

7 years ago
Comment on attachment 537573 [details] [diff] [review]
Patch to move bugzilla.dtd to template (v1)

>=== added file 'bugzilla_dtd.cgi'

  Since it's just static, how about making it a page.cgi page?

>=== added file 'template/en/default/bug/bugzilla_dtd.txt.tmpl'

  The .txt will cause it to be served as text/plain. I suspect that is not the correct MIME type for an XML DTD? Perhaps we need a new MIME type and extension. (I believe they are in Bugzilla::Constants.)

>+[% FOREACH field = Bugzilla.active_custom_fields %]
>+                [%+ field.name FILTER none %]?, 

  Maybe FILTER xml? (Field names certainly can't *currently* contain anything dangerous, but it doesn't hurt to be safe here.)

>+[% FOREACH field = Bugzilla.active_custom_fields %]
>+<!ELEMENT [% field.name FILTER none %] (#PCDATA)>
>+[% END %]

  My only thought here is for multi-select fields, will they require custom code?

>=== modified file 'template/en/default/bug/show.xml.tmpl'
>+<!DOCTYPE bugzilla [% IF cgi.param('dtd') %][[% PROCESS bug/bugzilla_dtd.txt.tmpl %]][% ELSE %]SYSTEM "[% urlbase FILTER html %]bugzilla_dtd.cgi"[% END 

  This changes the default behavior of an interface that is used programmatically in applications that operate across a lot of Bugzilla versions. Perhaps instead, if "dtd" isn't defined, default it to 1? (And then people have to specify &dtd=0 directly to make it SYSTEM.)

  I do agree that SYSTEM is certainly simpler though....
Attachment #537573 - Flags: review?(mkanat) → review-

Comment 17

7 years ago
Oh wait, nevermind my comment about SYSTEM and &dtd. That is totally fine, you did preserve the current behavior, I just misread the code. :-) Keep it how you wrote it, I like that. :-)
(Assignee)

Comment 18

7 years ago
Created attachment 548479 [details] [diff] [review]
Patch to move bugzilla.dtd to template (v2)

Thanks for the review. Here is a revised patch with your suggested changes.

dkl
Attachment #537573 - Attachment is obsolete: true
Attachment #548479 - Flags: review?(mkanat)

Comment 19

7 years ago
Comment on attachment 548479 [details] [diff] [review]
Patch to move bugzilla.dtd to template (v2)

Review of attachment 548479 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great! A few things to fix on checkin:

::: Bugzilla/Constants.pm
@@ +441,5 @@
> +   "html"   => "text/html" ,
> +   "rdf"    => "application/rdf+xml" ,
> +   "atom"   => "application/atom+xml" ,
> +   "xml"    => "application/xml" ,
> +   "xmldtd" => "application/xml-dtd" , 

Let's just make the extension dtd.

::: template/en/default/bug/show.xml.tmpl
@@ +25,2 @@
>  <?xml version="1.0" [% IF Param('utf8') %]encoding="UTF-8" [% END %]standalone="yes" ?>
> +<!DOCTYPE bugzilla [% IF cgi.param('dtd') %][[% PROCESS pages/bugzilla_dtd.xmldtd.tmpl %]][% ELSE %]SYSTEM "[% urlbase FILTER html %]page.cgi?id=bugzilla_dtd.xmldtd"[% END %]>

That should be FILTER xml, not html. (I know it was html before, but that's wrong.)

::: template/en/default/pages/bugzilla_dtd.xmldtd.tmpl
@@ +14,5 @@
> +  # Corporation. Portions created by Netscape are
> +  # Copyright (C) 1998 Netscape Communications Corporation. All
> +  # Rights Reserved.
> +  #
> +  # Contributor(s): David Lawrence <dkl@mozilla.com>

If you're the only contributor, the Initial Developer block above can't be right.
Attachment #548479 - Flags: review?(mkanat) → review+

Updated

7 years ago
Flags: approval?
Target Milestone: --- → Bugzilla 5.0
(Assignee)

Comment 20

7 years ago
Created attachment 550474 [details] [diff] [review]
Patch to move bugzilla.dtd to template (v3)

Here is the revised patch with suggested changes. Moving over r+ and waiting on 5.0 development to commit.

1. Updated contributor list to include those who have worked on the original bugzilla.dtd
2. Put certain *bug* words in " " in pages/bugzilla.dtd.tmpl to allow passing of t/009bugwords.t.

dkl
Attachment #548479 - Attachment is obsolete: true
Attachment #550474 - Flags: review+
(Assignee)

Updated

7 years ago
Whiteboard: [bmo4.0-resolved]

Updated

7 years ago
Flags: approval? → approval+
(Assignee)

Comment 21

7 years ago
Committing to: bzr+ssh://dlawrence%40mozilla.com@bzr.mozilla.org/bugzilla/trunk
missing bugzilla.dtd
modified bugzilla.dtd
modified Bugzilla/Constants.pm
modified template/en/default/bug/show.xml.tmpl
added template/en/default/pages/bugzilla.dtd.tmpl
Committed revision 7900.

dkl
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Keywords: relnote

Comment 22

6 years ago
Added to relnotes for 4.4.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.