Edgewall Software
Modify

Opened 6 years ago

Closed 4 years ago

Last modified 4 years ago

#10841 closed defect (fixed)

Less broad CSS rules for styling ticket #properties table

Reported by: Ethan Jucovy <ethan.jucovy@…> Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.4
Component: rendering Version:
Severity: normal Keywords:
Cc:
Release Notes:
API Changes:

On the ticket page, the CSS selectors descending from the #properties table are more precise.

Description

I've set up a Trac site that loads a Javascript WYSIWYG editor to edit the ticket description field. The WYSIWYG editor creates a <table> to present a toolbar to the user.

Trac's ticket.css contains rules for #properties table which make the layout of this toolbar table look wrong, because it's a <table> that's a descendent of the <fieldset id="properties">. So, in order to make the WYSIWYG toolbar display properly, I need to track down all the #properties table rules and suppress them with a more specific CSS selector in my own stylesheet.

However, Trac's CSS rule is unnecessarily broad. The only table that the CSS is trying to style is a direct child of the properties fieldset, so using selectors like #properties > table instead of #properties table would have the same effect without accidentally styling other tables that might be inserted within the #properties node. I believe the CSS child selector has been supported in all browsers for a while now (IE7+).

Attachments (3)

ticket_css_propertiestable.diff (1.0 KB ) - added by Ethan Jucovy <ethan.jucovy@…> 6 years ago.
ticket_css_propertiestable_without_child_selector.diff (1.7 KB ) - added by Ethan Jucovy <ethan.jucovy@…> 6 years ago.
ticket_css_propertiestable_multiple_child_selectors.diff (1.1 KB ) - added by Ethan Jucovy <ethan.jucovy@…> 6 years ago.
Use multiple child selectors in each rule (table > tbody > tr etc)

Download all attachments as: .zip

Change History (20)

Changed 6 years ago by Ethan Jucovy <ethan.jucovy@…>

Changed 6 years ago by Ethan Jucovy <ethan.jucovy@…>

comment:1 Changed 6 years ago by Ethan Jucovy <ethan.jucovy@…>

If using the CSS child selector is problematic, the same effect could be achieved by adding a class or ID to the relevant table to make the selector more specific. I'm not sure which approach is considered better, so I've attached two patches.

Changed 6 years ago by Ethan Jucovy <ethan.jucovy@…>

Use multiple child selectors in each rule (table > tbody > tr etc)

comment:2 Changed 6 years ago by Ethan Jucovy <ethan.jucovy@…>

Actually neither of my first two patches is sufficient, because of course the inner table's <th>s and <td>s are still caught by the rule. attachment:ticket_css_propertiestable_multiple_child_selectors.diff fixes this by using a lot of child selectors (e.g. #properties > table > tbody > tr > th) which makes the application of the rules sufficiently specific.

comment:3 Changed 6 years ago by Christian Boos

I didn't look at the specifics of the problem yet, but just seeing your latest patch, I think you should aim for simple and efficient CSS by adding whatever is required in terms of IDs or classes (trac- prefix), rather than trying to cope with the existing structure.

comment:4 Changed 6 years ago by Christian Boos

Milestone: undecided

All the tickets for {20} from last year have probably been seen multiple times by now, yet are still to be triaged…

comment:5 Changed 4 years ago by Ryan J Ollos

I took the suggestions from this ticket and prepared the changes in log:rjollos.git:t10841. Ethan, does it solve the issue you were encountering?

comment:6 Changed 4 years ago by Peter Suter

Closely related to #11896.

comment:7 Changed 4 years ago by Ryan J Ollos

Milestone: undecided1.0.4
Owner: set to Ryan J Ollos
Status: newassigned

comment:8 Changed 4 years ago by Ryan J Ollos

Milestone: 1.0.41.0.5

comment:9 Changed 4 years ago by Ryan J Ollos

Tentative plan is to just apply attachment:ticket_css_propertiestable_multiple_child_selectors.diff, since it's similar to the change in [13662] and will get the job done for now. However, I haven't fully tested that patch yet.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:10 Changed 4 years ago by Ryan J Ollos

Milestone: 1.0.51.1.4

There will be less risk to apply to trunk.

comment:11 Changed 4 years ago by Ryan J Ollos

API Changes: modified (diff)

Proposed changes in log:rjollos.git:t10841.2.

comment:12 Changed 4 years ago by Ryan J Ollos

Resolution: fixed
Status: assignedclosed

Committed to trunk in [13754].

comment:13 Changed 4 years ago by Ryan J Ollos

API Changes: modified (diff)

comment:14 Changed 4 years ago by Jun Omae

I noticed a similar issue in trunk. Wiki table in ticket comment in auto preview is rendered as width: 100%. Wiki table usually is rendered as width: auto.

Last edited 4 years ago by Jun Omae (previous) (diff)

comment:15 Changed 4 years ago by Ryan J Ollos

That might be a regression in [13211]. This patch seems to fix the issue.

  • trac/htdocs/css/ticket.css

    diff --git a/trac/htdocs/css/ticket.css b/trac/htdocs/css/ticket.css
    index d442945..1fd8660 100644
    a b form .field fieldset { margin-left: 1px; margin-right: 1px  
    204204label[for=comment] { float: right }
    205205
    206206#propertyform { margin-bottom: 2em; }
    207 #propertyform table {
     207#propertyform table.trac-properties {
    208208 border-spacing: 0;
    209209 table-layout: fixed;
    210210 width: 100%;
    211211}
    212 #propertyform table td input[type="text"],
    213 #propertyform table td textarea {
     212#propertyform table.trac-properties td input[type="text"],
     213#propertyform table.trac-properties td textarea {
    214214 -moz-box-sizing: border-box;
    215215 box-sizing: border-box;
    216216 width: 100%;
  • trac/ticket/templates/ticket.html

    diff --git a/trac/ticket/templates/ticket.html b/trac/ticket/templates/ticket.ht
    index 98bd234..76c5f2f 100644
    a b ${value}</textarea>  
    327327            <!--! Comment field -->
    328328            <fieldset>
    329329              <div py:if="authname == 'anonymous'" class="author">
    330                 <table>
     330                <table class="trac-properties">
    331331                  <tr>
    332332                    <th>
    333333                      <label for="author">Your email or username:</label><br />

comment:16 in reply to:  15 Changed 4 years ago by Jun Omae

Replying to rjollos:

That might be a regression in [13211]. This patch seems to fix the issue.

Thanks. That looks good to me. Please push it.

comment:17 Changed 4 years ago by Ryan J Ollos

Thanks for the review. Committed to trunk in [13781].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.