Closed Bug 813725 Opened 9 years ago Closed 8 years ago

Top of RSS item viewing window frozen


(Thunderbird :: Folder and Message Lists, defect)

17 Branch
Not set


(Not tracked)

Thunderbird 23.0


(Reporter: epaul137ml, Assigned: alta88)


(Keywords: regression)


(1 file, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20121119183901

Steps to reproduce:

I opened a RSS entry viewing window by double clicking on a RSS item. This displayed the RSS item in a new window.

Actual results:

After viewing and deleting the RSS item, the bottom of the window updated to display the next RSS item but the top of the window is still showing the title and http link to the item that was deleted. After viewing and deleting multiple items, the top of window still displays the information from the first item that was displayed in the window. Closing the window and doubling clicking the next RSS item, starts the issue over.

Expected results:

The top of the window should follow the bottom of the window. The window operated correctly in Thunderbird 16 and below.
Closed: 9 years ago
Resolution: --- → INVALID
The problem only occurs if you reading RSS items from the top down. If you are reading items from the bottom up, the top of the window updates properly.
Resolution: INVALID → ---
hmm, this happens on message deletion in either the standalone window or 3pane (regardless if the message opened initially is first or last in threadpane) only if Message->Open Feed message is Web Page.  the standalone window headers never update even though the right web page loads (for next message after deletion).
Ever confirmed: true
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → alta88
Attachment #741662 - Flags: review?(mkmelin+mozilla)
Comment on attachment 741662 [details] [diff] [review]

Review of attachment 741662 [details] [diff] [review]:

::: mail/base/content/folderDisplay.js
@@ +1262,5 @@
>      let viewIndex = this.view.dbView.currentlyDisplayedMessage;
>      let msgHdr = (viewIndex != nsMsgViewIndex_None) ?
>                     this.view.dbView.getMsgHdrAt(viewIndex) : null;
> +    if ((this instanceof FolderDisplayWidget) &&

I don't think it's acceptable to do instanceof here. 
Override in StandaloneFolderDisplayWidget as needed instead?
Attachment #741662 - Flags: review?(mkmelin+mozilla) → review-
OS: Windows 7 → All
Hardware: x86 → All
Keywords: regression
why exactly?  StandaloneFolderDisplayWidget calls FolderDisplayWidget proto.  having to recreate the displayMessageChanged() function in SFDW without these 2 lines seems quite inefficient.
StandaloneFolderDisplayWidget is supposed to handle standalone window differences from FolderDisplayWidget. If we start testing for standalone inside FolderDisplayWidget it's a slippery slope... 

You don't necessarily need to copy the implementation. You could create a helper method in FolderDisplayWidget that handles setting the content, then override that metod in StandaloneFolderDisplayWidget to do nothing. 

Or maybe there's better ways - i'm not sure i understand what the problem was?
Attached patch patch (obsolete) — Splinter Review
well, i guess i still don't understand why it's (too) slippery.  yes, there are numerous other methods, but there are 2 subclasses of FDW in the codebase, and i wanted to do it in < 1 line.. ;)

fortunately it turns out the subclasses already call the super with a null aTabInfo (for the same reasons) and this will work perfectly (better).
Attachment #741662 - Attachment is obsolete: true
Attachment #742368 - Flags: review?(mkmelin+mozilla)
Comment on attachment 742368 [details] [diff] [review]

Review of attachment 742368 [details] [diff] [review]:

Seems to be some problem with the diff

$ hg qpush -v
applying feedWindow.patch
patching file mail/base/content/folderDisplay.js
bad hunk #1 @@ -1258,17 +1258,20 @@ FolderDisplayWidget.prototype = {
 (18 17 20 20)
patch failed, rejects left in working dir
errors during apply, please fix and refresh feedWindow.patch
Attachment #742368 - Flags: review?(mkmelin+mozilla)
Attached patch updated (obsolete) — Splinter Review
Attachment #742368 - Attachment is obsolete: true
Attachment #742505 - Flags: review?(mkmelin+mozilla)
Comment on attachment 742505 [details] [diff] [review]

Review of attachment 742505 [details] [diff] [review]:

::: mail/base/content/folderDisplay.js
@@ +1269,1 @@
>        FeedMessageHandler.setContent(msgHdr, false);

I think this if should have braces since thers's the comment in there.

also ie -> i.e. and "a tabInfo"
Attachment #742505 - Flags: review?(mkmelin+mozilla) → review+
Attached patch for checkinSplinter Review
Attachment #742505 - Attachment is obsolete: true
Attachment #742535 - Flags: review+
Keywords: checkin-needed
Closed: 9 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0
will this be deployed to earlybird or beta soon?
or this will be known issue until tb23?
I'd like to check out Bug 816414 which is said to have this as root cause.
not sure if I should wait for tb22 beta upon code merge or nightly directly.
Component: Untriaged → Folder and Message Lists
You need to log in before you can comment on or make changes to this bug.