Modify

Opened 13 years ago

Closed 12 years ago

#8628 closed defect (fixed)

[Patch] bookmark.url not being expanded correctly on sub sites

Reported by: branson Owned by: yosiyuki
Priority: normal Component: BookmarkPlugin
Severity: major Keywords:
Cc: melissa.k.mcdowell@… Trac Release: 0.12

Description

  • I have a site at /trac/atol
  • have the most recent bookmarks plugin
  • we are seeing this:
    <ul class="bookmarks">
        <li class="report">
            <a href="/report/8">{8}</a>
                Active Tickets, Mine first (<a href="/trac/atol/bookmark/delete_in_page/report/8">delete</a>)
        </li>
    </ul>

for code that looks like this in the list.html template:

 <ul class="bookmarks">
      <py:for each="bookmark in bookmarks">
        <li class="${bookmark.realm}">
            <a href="${bookmark.url}">${bookmark.linkname}</a>
                ${bookmark.name} (<a href="${bookmark.delete}">delete</a>)
        </li>
      </py:for>
    </ul>

appears that the bookmark.url is not being expanded correctly.

Attachments (1)

bookmark.patch (663 bytes) - added by Bangyou Zheng 13 years ago.

Download all attachments as: .zip

Change History (9)

Changed 13 years ago by Bangyou Zheng

Attachment: bookmark.patch added

comment:1 Changed 12 years ago by Ryan J Ollos

Summary: bookmark.url not being expanded correctly on sub sites.[Patch] bookmark.url not being expanded correctly on sub sites

comment:2 Changed 12 years ago by Ryan J Ollos

#9788, which I opened, is a duplicate.

comment:3 Changed 12 years ago by Jun Omae

Resolution: fixed
Status: newclosed

(In [11271]) Fixed generating wrong bookmark urls if base-path isn't /.

Initial patch by sandinak. Closes #8628.

comment:4 Changed 12 years ago by Ryan J Ollos

Hi jun66j5,

Did you test that patch on bookmarks with hrefs that contain query strings? I did some testing on hrefs that contain query strings in the course of my work on #7399, and then again while starting to work on #9788, and found that using req.href(url) doesn't work for query strings (more details in #9788). I was thinking that req.base_path + url might work better here. I implemented a similar solution for the MenusPlugin in [11166]. I was thinking that a cleaner implementation there might be req.base_path + url.lstrip('/'), but I haven't tried that out yet.

comment:5 Changed 12 years ago by Jun Omae

Resolution: fixed
Status: closedreopened

Oh..., you're right. I try now and it doesn't work on url with query string.

comment:6 Changed 12 years ago by Ryan J Ollos

I've seen a similar issue with at least 3 other Trac plugins, so I'll be interested to see what solution you implement since I think you know the Trac API much better than I do. It would be really nice if req.href(..) could just handle query arguments and fragments though.

comment:7 Changed 12 years ago by Ryan J Ollos

Also, I can't say for certain, but think that #8353 was probably resolved by this patch as well. At least, I think I may have experienced that issue yesterday when working in a dev instance of Trac, and prior to this patch.

comment:8 Changed 12 years ago by Jun Omae

Resolution: fixed
Status: reopenedclosed

(In [11272]) Follow-up to [11271], fixed generating wrong link when a page with query string is bookmarked.

Closes #8628.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain yosiyuki.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.