Modify

Opened 3 years ago

Closed 2 years ago

#8628 closed defect (fixed)

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

Reported by: sandinak Owned by: saigon
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 helend 3 years ago.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by helend

comment:1 Changed 2 years ago by rjollos

  • Summary changed from bookmark.url not being expanded correctly on sub sites. to [Patch] bookmark.url not being expanded correctly on sub sites

comment:2 Changed 2 years ago by rjollos

#9788, which I opened, is a duplicate.

comment:3 Changed 2 years ago by jun66j5

  • Resolution set to fixed
  • Status changed from new to closed

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

Initial patch by sandinak. Closes #8628.

comment:4 Changed 2 years ago by rjollos

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 2 years ago by jun66j5

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:6 Changed 2 years ago by rjollos

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 2 years ago by rjollos

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 2 years ago by jun66j5

  • Resolution set to fixed
  • Status changed from reopened to closed

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

Closes #8628.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from saigon. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.