Closed Bug 1279440 Opened 3 years ago Closed 3 years ago

When refresh webmail page in FF 47, it shows previous page plus one (incremental)

Categories

(Core :: DOM: Core & HTML, defect)

47 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla50
Tracking Status
firefox47 - wontfix
firefox48 + verified
firefox49 + verified
firefox50 + verified

People

(Reporter: pencov, Assigned: dragana)

References

Details

(Keywords: regression, Whiteboard: btpp-active)

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

Until this version: 47, we have not this issue.
We are using Horde webmail system and if in some folders (Inbox par example) you have a lot of emails you have multiple pages/lists so you can go to next page from the list if you want. And always, when you go back into this folder, it shows you the last page you´v been. OS is Windows 7 64 bits. Firefox buildconfig is x86_64-pc-mingw32.


Actual results:

When you access a folder with list of emails with multiple pages, every time it's showing you next page. Like it's increment it. 
Let's say you have 3 pages in Inbox (hundred of emails). When first time you go into Inbox, it shows you 1st page (like it should happen). When you go into other folder and turn back in Inbox it showing you 2nd page (but should display 1st or last page you´v been). Again, when you go into other folder and go back in Inbox, it´s showing you 3rd page. And so on!
This is happening in every folder where you have multiple pages.


Expected results:

In multiple pages should always show last page where you´v been last time. Not to increment pages
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
Could you provide a testcase, maybe a guest Horde email account with 2 or 3 pages of random emails.

It will hard to debug without something to test.
Flags: needinfo?(pencov)
(In reply to Loic from comment #1)
> Could you provide a testcase, maybe a guest Horde email account with 2 or 3
> pages of random emails.
> 
> It will hard to debug without something to test.

Hi,
Here are information for you to login with this test account.
You will find in the Inbox 48 messages (5 pages)
To simulate the problem, just click on Drafts folder and back on Inbox couple times.
Be aware that we have limitation for attachments - no bigger than 3MB.

host: https://mail.artmatch.ro <<-- accept certificate
u: tset.bugzilla@artmatch.ro
p: aKP8tJfhgXq.D8

I´m looking your result about the test.

Thank you in advance,
Cristian
Flags: needinfo?(pencov)
Hi Cristian,

I tested this issue on Mac OS X 10.10 with FF 50.0a1 and FF 47 release and I can reproduce it, but with FF 46 release I wasn't able to reproduce it. This is a regression, I used mozregression and here are the results: 

Last good revision: 346a7684fde5373adf47175a61d312d536e833c5
First bad revision: 5b20181a5b50b92259038a88202b2ece37e2b702
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=346a7684fde5373adf47175a61d312d536e833c5&tochange=5b20181a5b50b92259038a88202b2ece37e2b702

Based on the results from mozregression I think the culprit bug is https://bugzilla.mozilla.org/show_bug.cgi?id=580313, this is also the reason for assigning to Core: DOM.
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM
Ever confirmed: true
Keywords: regression
OS: Windows 7 → All
Product: Firefox → Core
Hardware: x86_64 → All
Version: 47 Branch → 50 Branch
It's reproducible in 50 too.
Blocks: 580313
Has Regression Range: --- → yes
Has STR: --- → yes
Flags: needinfo?(dd.mozilla)
Version: 50 Branch → 47 Branch
I had a look and it seems like an application problem.

So we send a request for https://mail.artmatch.ro/imp/mailbox.php?no_newmail_popup=1&mailbox=INBOX
it  returns response. The response has a Link element with rel="Next" so we do prefetch here (this is change made in bug 580313). So we do prefetch of https://mail.artmatch.ro/imp/mailbox.php?mailbox=INBOX&page=2
When we go back to the INBOX, firefox is sending again a request for https://mail.artmatch.ro/imp/mailbox.php?no_newmail_popup=1&mailbox=INBOX. It checks the cache and decide to do fetch again (from the log "Must validate since response contains 'no-cache' header").
The response is 200 OK but the content of the page is actually content for page 2 not 1.
Flags: needinfo?(dd.mozilla)
Based on Dragana's investigation this may not be an issue on our side. However, even if this was a valid issue, I do not think it is critical enough to be included in a dot release as a ride-along.
Folks,
I´m very sorry, but based on Ritu´s proposal/decision to not fix this issue, I must switch to another browser.
I will not leave >50 people to use Firefox with this issue because is affecting our business.
Beside this example with jumping to next page (let's say we can live with it) there is a critical one witch we can not live with it.

Explanation: when you have in Inbox many Unread emails and you open one, after you return to Inbox the next one in the list (i suppose) is marked as Read/Seen.

So, bottom of the line, I need confirmation that I do not have any solution [patch, addons etc] for this problems except to stitch to another browser.

Thank you all for your involvement.
Cristian
Hi Cristian,

You can use FF 46 version. You said that with Firefox 46 you didn't had any problems. If you think this is a good solution for you and your company, you can download FF 46 from here: http://archive.mozilla.org/pub/firefox/releases/46.0/win64/en-US/
we use rel"next" in the same way as prefetch. I have not change this in my patch.
Chrome does not prefetch rel="next" and therefor does not have this problem.

smaug, should we do something here?
Flags: needinfo?(bugs)
So what in bug 580313 exactly causes the regression?
Flags: needinfo?(bugs)
(In reply to ovidiu boca[:Ovidiu] from comment #8)
> Hi Cristian,
> 
> You can use FF 46 version. You said that with Firefox 46 you didn't had any
> problems. If you think this is a good solution for you and your company, you
> can download FF 46 from here:
> http://archive.mozilla.org/pub/firefox/releases/46.0/win64/en-US/

Ovidiu,
you are right, this was my 1st solution after discovering the problem, but what about staying vulnerable to security holes because I can not update FF 46 to latest version?! 
Honestly, I do not see this solution to work but just temporally and only if I know that the problem will be solved let's say next month or with release 51 :)
I really want to know what final decision to take, so I´m waiting last word from developers about this.
Thank you anyway for your solution.
We used to support "next" even before bug 580313, so is it that we now support dynamic changes which causes the issue?

(and yes, this sounds like an issue in the page, but it is possible that we need to revert our "next" handling to the behavior we used to have)
Flags: needinfo?(dd.mozilla)
The change that causes the regression is:
https://dxr.mozilla.org/mozilla-central/source/dom/html/HTMLLinkElement.cpp#172

in HTMLLinkElement we used to do preconnect and dns_prefetch but not rel=prefetch or rel=next

We could remove eNext for this case.

I think the link is not dynamically added.
Flags: needinfo?(dd.mozilla)
But we did support "next" even before bug 580313. So what caused the regression?
The patch removed code in parser/html/nsHtml5DocumentBuilder.cpp which dealt with "next", and moved it to be handled the same way as attributes usually are handled.
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Attachment #8764633 - Flags: review?(bugs)
Oh, we had
bool hasPrefetch = linkTypes & nsStyleLinkElement::ePREFETCH;
PrefetchHref(hrefVal, aElement, hasPrefetch);
in html5documentbuilder.

I assume you tested the patch :)
Attachment #8764633 - Flags: review?(bugs) → review+
We want the fix in branches, right? Perhaps even in release if we get some . updates?
Whiteboard: btpp-active
(In reply to Olli Pettay [:smaug] (high review load, please consider other reviewers) from comment #17)
> Oh, we had
> bool hasPrefetch = linkTypes & nsStyleLinkElement::ePREFETCH;
> PrefetchHref(hrefVal, aElement, hasPrefetch);
> in html5documentbuilder.
> 
> I assume you tested the patch :)

I have tested the patch :)
Keywords: checkin-needed
Comment on attachment 8764633 [details] [diff] [review]
bug_1279440.patch

Approval Request Comment
[Feature/regressing bug #]: 580313
[User impact if declined]:See comment 0 and comment 7. I do not know how spread this is, but is only one line change
[Describe test coverage new/current, TreeHerder]: Manually tested case in comment 0 and there are tests introduced in bug 580313
[Risks and why]: none
[String/UUID change made/needed]: none
Attachment #8764633 - Flags: approval-mozilla-release?
Attachment #8764633 - Flags: approval-mozilla-beta?
Attachment #8764633 - Flags: approval-mozilla-aurora?
(In reply to Cristian-Petru Pencov from comment #11)
> (In reply to ovidiu boca[:Ovidiu] from comment #8)
> > Hi Cristian,
> > 
> > You can use FF 46 version. You said that with Firefox 46 you didn't had any
> > problems. If you think this is a good solution for you and your company, you
> > can download FF 46 from here:
> > http://archive.mozilla.org/pub/firefox/releases/46.0/win64/en-US/
> 
> Ovidiu,
> you are right, this was my 1st solution after discovering the problem, but
> what about staying vulnerable to security holes because I can not update FF
> 46 to latest version?! 
> Honestly, I do not see this solution to work but just temporally and only if
> I know that the problem will be solved let's say next month or with release
> 51 :)
> I really want to know what final decision to take, so I´m waiting last word
> from developers about this.
> Thank you anyway for your solution.

It is very small change so it most probably will be uplifted to FF48(I am not making that decision but I expect to be uplifted). About release (47), I cannot tell there is a process for that and corresponding people need to evaluate it.
(In reply to Dragana Damjanovic [:dragana] from comment #22)
> (In reply to Cristian-Petru Pencov from comment #11)
> > (In reply to ovidiu boca[:Ovidiu] from comment #8)
> > > Hi Cristian,
> > > 
> > > You can use FF 46 version. You said that with Firefox 46 you didn't had any
> > > problems. If you think this is a good solution for you and your company, you
> > > can download FF 46 from here:
> > > http://archive.mozilla.org/pub/firefox/releases/46.0/win64/en-US/
> > 
> > Ovidiu,
> > you are right, this was my 1st solution after discovering the problem, but
> > what about staying vulnerable to security holes because I can not update FF
> > 46 to latest version?! 
> > Honestly, I do not see this solution to work but just temporally and only if
> > I know that the problem will be solved let's say next month or with release
> > 51 :)
> > I really want to know what final decision to take, so I´m waiting last word
> > from developers about this.
> > Thank you anyway for your solution.
> 
> It is very small change so it most probably will be uplifted to FF48(I am
> not making that decision but I expect to be uplifted). About release (47), I
> cannot tell there is a process for that and corresponding people need to
> evaluate it.

That's great news and I hope it will be solved in FF48. I will follow this thread until it's Closed.
Have a nice day all and thanks again.
https://hg.mozilla.org/mozilla-central/rev/3105928e631b
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment on attachment 8764633 [details] [diff] [review]
bug_1279440.patch

No go for inclusion in 47 dot release.
Attachment #8764633 - Flags: approval-mozilla-release? → approval-mozilla-release-
Verified on Nightly 50.0a1 (2016-06-26) with Mac OS X 10.10. Everything works as expected, the issue can't be reproduce.
Comment on attachment 8764633 [details] [diff] [review]
bug_1279440.patch

Review of attachment 8764633 [details] [diff] [review]:
-----------------------------------------------------------------

This patch fixes the regression and is verified. Take it in 48 beta 4 and aurora.
Attachment #8764633 - Flags: approval-mozilla-beta?
Attachment #8764633 - Flags: approval-mozilla-beta+
Attachment #8764633 - Flags: approval-mozilla-aurora?
Attachment #8764633 - Flags: approval-mozilla-aurora+
Tracking 48/49/50+ for this regression - we need to make sure pages are not incremented in this case.
I verified this issue on Nightly 50.0a1 (2016-06-28), Firefox aurora 49 (2016-06-28) and FF beta 48.0b4 build1 and I can't reproduce this issue.
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.