Opened 10 years ago

Closed 10 years ago

#4956 closed bug (fixed)

Previewing a ticket shows empty Description box

Reported by: humdinger Owned by: nielx
Priority: high Milestone: R1
Component: Website/Trac Version: R1/alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

Since the move, the description box is empty when doing a "Preview". That means you cannot correct any errors and have to use the browser back button which will empty the drop-down menus for "Component" etc.

Misusing this ticket: The "Description:" label is in front of the icon shortcut bar. Should be one line down.

Change History (14)

comment:1 by humdinger, 10 years ago

Priority: normalhigh

To add: Danger, Will Robinson'''

It you don't use the back-button and instead submit directly from the Preview page (maybe because the text in the preview pane was satisfactory) the description will be submitted empty!

comment:2 by zooey, 10 years ago

Owner: changed from haiku-web to zooey
Status: newassigned

Looking into it now - I couldn't reproduce this when I tried earlier, but could now with a fresh ticket.

comment:3 by zooey, 10 years ago

Resolution: fixed
Status: assignedclosed

Fixed by removing an apparently broken fragment from the site.html template - as a result, the note about wiki-formatting being allowed in the textarea is currently missing.

But I'm working on getting trac patches in via another way, so that note will come back, later.

comment:4 by zooey, 10 years ago

The note is back, now that I have installed the new (our own) trac package - let's see if the author name appears in notifications resulting from this comment ...

comment:5 by humdinger, 10 years ago

It does. Well done, thanks!

comment:6 by nielx, 10 years ago

Resolution: fixed
Status: closedreopened

This does have to be fixed in a better way than a hacked Trac installation. Reopening to track the proper fix.

comment:7 by nielx, 10 years ago

Owner: changed from zooey to nielx
Status: reopenednew

comment:8 by nielx, 10 years ago

Owner: changed from nielx to zooey

I fixed the site.html template. Oliver, (if you agree with my proposal on handling Trac modifications), could you revert the patch to the Trac sources and enable the new site.html on dev.haiku-os.org?

comment:9 by nielx, 10 years ago

Forgot to mention: tested on a local Trac (0.11.6pre) installation.

in reply to:  6 ; comment:10 by zooey, 10 years ago

Status: newassigned

Replying to nielx:

This does have to be fixed in a better way than a hacked Trac installation. Reopening to track the proper fix.

Niels, I must say that I don't like the tone of your comment above: disregarding my solution to the bug in site.html as 'a hacked trac installation' surely doesn't go down well with me :-(

The result of the bug was pretty severe, so something had to be done. And, honestly, whether or not changing the original template file within the trac package is worse than patching the same template file on the go via site.html, is at least debatable. My personal view on this is that trac's template-patching mechanism is neither easy to grok nor will it scale well with many changes, so I actually prefer adjusting the template files themselves to our needs. But that's just my view - I may be wrong ...

But, anyway, since you are the trac-admin, I will move the patch out of the package back into site.html.

in reply to:  10 ; comment:11 by nielx, 10 years ago

Replying to zooey:

Replying to nielx:

This does have to be fixed in a better way than a hacked Trac installation. Reopening to track the proper fix.

Niels, I must say that I don't like the tone of your comment above: disregarding my solution to the bug in site.html as 'a hacked trac installation' surely doesn't go down well with me :-(

I think you are misinterpreting the tone. Making modifications to the default Trac sources is hacking. The critical note was based on that I think it is the wrong way to go about fixing things, not towards you personally. I understand that it was a critical bug and that it must be fixed.

The result of the bug was pretty severe, so something had to be done. And, honestly, whether or not changing the original template file within the trac package is worse than patching the same template file on the go via site.html, is at least debatable. My personal view on this is that trac's template-patching mechanism is neither easy to grok nor will it scale well with many changes, so I actually prefer adjusting the template files themselves to our needs. But that's just my view - I may be wrong ...

Well, my view is different (see message on haiku-sysadmin). The summary (for those not following the list): I suggest separating the hack logic from the Trac logic (through plugins and template customization) wherever possible.

But, anyway, since you are the trac-admin, I will move the patch out of the package back into site.html.

Thanks, but I hope you are not just doing this because I am the trac-admin.

Final note: in case of future template modification that does not go through site.html, it is also possible to copy the original template to the env's template dir and make the changes there. It will be picked up then as well.

in reply to:  11 ; comment:12 by zooey, 10 years ago

Owner: changed from zooey to nielx
Status: assignednew

Replying to nielx:

Replying to zooey:

Niels, I must say that I don't like the tone of your comment above: disregarding my solution to the bug in site.html as 'a hacked trac installation' surely doesn't go down well with me :-(

I think you are misinterpreting the tone.

Sigh, thanks for being so considerate of my feelings ...

The result of the bug was pretty severe, so something had to be done. And, honestly, whether or not changing the original template file within the trac package is worse than patching the same template file on the go via site.html, is at least debatable. My personal view on this is that trac's template-patching mechanism is neither easy to grok nor will it scale well with many changes, so I actually prefer adjusting the template files themselves to our needs. But that's just my view - I may be wrong ...

Well, my view is different (see message on haiku-sysadmin). The summary (for those not following the list): I suggest separating the hack logic from the Trac logic (through plugins and template customization) wherever possible.

But separation has to be weighed against reliability: if keeping things separate gives a less reliable result, maybe it's not worth it.

But, anyway, since you are the trac-admin, I will move the patch out of the package back into site.html.

Thanks, but I hope you are not just doing this because I am the trac-admin.

What other reason would there be? I have not heard of any evidence that doing it your way is better than doing it mine. You keep stating it, but that hasn't convinced me yet.

I have now removed the patch from the package again and restored site.html and adjusted it according to your patch. It works, but due to how the matching in site.html is done, the note is shown only for new tickets, it is not being shown when you're editing or previewing an existing ticket.

This wasn't the case before (i.e. the note was always shown). I know this can be fixed in site.html, but I guess this just goes to show that the whole on-the-fly-template-patching approach is somewhat complicated and hard to get right.

Niels: if you think the note should be visible in all contexts of the editable ticket view, please adjust site.html accordingly (or just close the ticket if it's ok as it is now).

Final note: in case of future template modification that does not go through site.html, it is also possible to copy the original template to the env's template dir and make the changes there. It will be picked up then as well.

I personally consider that the worst of the three approaches, as during an upgrade, it would be very easy to forget updating these templates and they'd silently override any changes in the newer trac version.

in reply to:  12 comment:13 by nielx, 10 years ago

Replying to zooey:

The result of the bug was pretty severe, so something had to be done. And, honestly, whether or not changing the original template file within the trac package is worse than patching the same template file on the go via site.html, is at least debatable. My personal view on this is that trac's template-patching mechanism is neither easy to grok nor will it scale well with many changes, so I actually prefer adjusting the template files themselves to our needs. But that's just my view - I may be wrong ...

Well, my view is different (see message on haiku-sysadmin). The summary (for those not following the list): I suggest separating the hack logic from the Trac logic (through plugins and template customization) wherever possible.

But separation has to be weighed against reliability: if keeping things separate gives a less reliable result, maybe it's not worth it.

That would be a good argument, but I am not quite sure wheter it applies here. There was a bug in the modification in site.html, now it is fixed. The syntax of the modification is not that complex and reasonably easy to understand if you know some XPath and Genshi.

But, anyway, since you are the trac-admin, I will move the patch out of the package back into site.html.

Thanks, but I hope you are not just doing this because I am the trac-admin.

What other reason would there be? I have not heard of any evidence that doing it your way is better than doing it mine. You keep stating it, but that hasn't convinced me yet.

I have now removed the patch from the package again and restored site.html and adjusted it according to your patch. It works, but due to how the matching in site.html is done, the note is shown only for new tickets, it is not being shown when you're editing or previewing an existing ticket.

This was a conscious choice. The original request was in #4121 and it only refered to the new ticket form. I did not think about extending this to the normal ticket pages.

This wasn't the case before (i.e. the note was always shown). I know this can be fixed in site.html, but I guess this just goes to show that the whole on-the-fly-template-patching approach is somewhat complicated and hard to get right.

Niels: if you think the note should be visible in all contexts of the editable ticket view, please adjust site.html accordingly (or just close the ticket if it's ok as it is now).

I'm okay with it either way. I think the rationale behind the original bug report was that the only ones who can change the description are developers, and they should be aware of the WikiFormatting, however new tickets can be created by everyone. Nonetheless, why not put it there at any time...

Final note: in case of future template modification that does not go through site.html, it is also possible to copy the original template to the env's template dir and make the changes there. It will be picked up then as well.

I personally consider that the worst of the three approaches, as during an upgrade, it would be very easy to forget updating these templates and they'd silently override any changes in the newer trac version.

Well, it does not really matter, in any upgrade we have to check whether the functionality of all modifications remain operational. As such we can store the diff of any modification in the patches in the trac-customizations module. The diff needs to be tested manually anyway, so whether it goes into the installed package itself, or the local templates dir does not really matter IMO.

Overall, you have to know that the Trac guys do not consider their internal classes (Ticket, Attachment, etc.) and their template data interfaces as stable APIs, rather they feel free to make modifications. This means that it is wise not only to test changes between major versions (0.11.x, 0.12.x) but also between minor versions (0.11.5, 0.11.6).

comment:14 by nielx, 10 years ago

Resolution: fixed
Status: newclosed

Okay, I decided not to implement it. I think developers (who are the only ones able to change the contents of the description box) would/should know the possibilities. Also, in Safari the line of text always seems a few pixels misaligned.

So closing for now.

Note: See TracTickets for help on using tickets.