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

Status: VERIFIED FIXED

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

Description

[Cristian-Petru Pencov]

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

Comment 1

[Loic]

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.

Comment 2

[Cristian-Petru 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: [e-mail]
p: aKP8tJfhgXq.D8

I´m looking your result about the test.

Thank you in advance,
Cristian

Comment 3

[ovidiu boca[:Ovidiu]]

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.

Comment 4

[Loic]

It's reproducible in 50 too.

Comment 5

[Dragana Damjanovic [:dragana]]

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.

Comment 6

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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.

Comment 7

[Cristian-Petru Pencov]

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

Comment 8

[ovidiu boca[:Ovidiu]]

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/

Comment 9

[Dragana Damjanovic [:dragana]]

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?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

So what in bug 580313 exactly causes the regression?

Comment 11

[Cristian-Petru Pencov]

(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.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

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)

Comment 13

[Dragana Damjanovic [:dragana]]

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.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

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.

Comment 15

[Dragana Damjanovic [:dragana]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8e880bd8176

Comment 16

[Dragana Damjanovic [:dragana]]

Attachment: bug_1279440.patch

Comment 17

[Olli Pettay [:smaug][[e-mail]]]

Oh, we had
bool hasPrefetch = linkTypes & nsStyleLinkElement::ePREFETCH;
PrefetchHref(hrefVal, aElement, hasPrefetch);
in html5documentbuilder.

I assume you tested the patch :)

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

We want the fix in branches, right? Perhaps even in release if we get some . updates?

Comment 19

[Dragana Damjanovic [:dragana]]

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

Comment 20

[Dragana Damjanovic [:dragana]]

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

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3105928e631b
Fix prefetch for rel=next. r=smaug

Comment 22

[Dragana Damjanovic [:dragana]]

(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.

Comment 23

[Cristian-Petru Pencov]

(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.

Comment 24

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3105928e631b

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8764633 [details] [diff] [review]
bug_1279440.patch

No go for inclusion in 47 dot release.

Comment 26

[ovidiu boca[:Ovidiu]]

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 27

[Gerry Chang [:gchang]]

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.

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ca386d377f0b

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ca386d377f0b

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8664ad5c5288

Comment 31

[Marcia Knous [:marcia]]

Tracking 48/49/50+ for this regression - we need to make sure pages are not incremented in this case.

Comment 32

[ovidiu boca[:Ovidiu]]

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.