Top of RSS item viewing window frozen

RESOLVED FIXED in Thunderbird 23.0

Status

Thunderbird
Folder and Message Lists
RESOLVED FIXED
5 years ago
9 months ago

People

(Reporter: epaul137ml, Assigned: alta88)

Tracking

({regression})

17 Branch
Thunderbird 23.0
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

3.68 KB, patch
alta88
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Status: UNCONFIRMED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → INVALID
(Reporter)

Comment 1

5 years ago
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.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
(Assignee)

Comment 2

4 years ago
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).
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 3

4 years ago
Created attachment 741662 [details] [diff] [review]
patch
Assignee: nobody → alta88
Attachment #741662 - Flags: review?(mkmelin+mozilla)

Comment 4

4 years ago
Comment on attachment 741662 [details] [diff] [review]
patch

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-

Updated

4 years ago
OS: Windows 7 → All
Hardware: x86 → All

Updated

4 years ago
Keywords: regression
(Assignee)

Comment 5

4 years ago
why exactly?  StandaloneFolderDisplayWidget calls FolderDisplayWidget proto.  having to recreate the displayMessageChanged() function in SFDW without these 2 lines seems quite inefficient.

Comment 6

4 years ago
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?
(Assignee)

Comment 7

4 years ago
Created attachment 742368 [details] [diff] [review]
patch

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 8

4 years ago
Comment on attachment 742368 [details] [diff] [review]
patch

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)
(Assignee)

Comment 9

4 years ago
Created attachment 742505 [details] [diff] [review]
updated


updated..
Attachment #742368 - Attachment is obsolete: true
Attachment #742505 - Flags: review?(mkmelin+mozilla)

Comment 10

4 years ago
Comment on attachment 742505 [details] [diff] [review]
updated

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+
(Assignee)

Comment 11

4 years ago
Created attachment 742535 [details] [diff] [review]
for checkin
Attachment #742505 - Attachment is obsolete: true
Attachment #742535 - Flags: review+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/13b289848f1d
Status: NEW → RESOLVED
Last Resolved: 5 years ago4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 23.0

Comment 13

4 years ago
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.