When trying to send/reply/forward an e-mail on webmail.earthlink.net I get a message "Please enter a URL"

VERIFIED FIXED in Firefox 50

Status

()

Core
Networking
VERIFIED FIXED
11 months ago
2 months ago

People

(Reporter: Mike, Assigned: valentin)

Tracking

({regression, site-compat})

50 Branch
mozilla52
x86_64
Windows 8.1
regression, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 unaffected, firefox50+ fixed, firefox51+ fixed, firefox52+ verified)

Details

(Whiteboard: [necko-active], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

(Reporter)

Description

11 months ago
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20160920155715

Steps to reproduce:

When logged into webmail.earthlink.net I create a message or hit reply to a message and it loads a page to create and send a message. I put in an address of the recipient, a subject, and type a message. Then I hit the "Send" button.


Actual results:

When I hit the "Send" box, all the boxes grey out and a pop-down/quote box comes down from the "Back" (left facing arrow) next to the address bar that says "Please enter a URL"


Expected results:

It should send the message and reload the previous screen with a message across the top that it successfully sent it.
(Reporter)

Updated

11 months ago
OS: Unspecified → Windows 8.1
Hardware: Unspecified → x86_64
(Reporter)

Comment 1

11 months ago
I have tried it on IE and it works there. I tried it in Firefox Safe Mode and it did the above thing. I also did a refresh of Firefox and it did the same. This has only been since the 50.0b1 and did not happen (to me) on 49 or below. I have not tried a complete reinstall, as it seems to be a bug and not an update issue at this point.

Here is a picture of what is happening when hitting the "Send" button:
http://technicaldifference.net/mikeiv/webmailerror.png

Comment 2

11 months ago
Maybe a regression in FF50+. Do you know if we can test https://webmail.earthlink.net/ with a guest account?
Flags: needinfo?(kjb1611mikeiv)
(Reporter)

Comment 3

11 months ago
Sure. Let me know when you are done with the account and I will delete it.

Address: bugzillatest@mindspring.com
Password: Pa55w0rd (Cap "P", lower "a", "fivefive", lower "w", "zero", lower "rd"

http://webmail.earthlink.net

Comment 4

11 months ago
Reg range: 
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a08e5896b83094df265f826091765d08a6e28b60&tochange=b31c2afa920a8f74ddddf0dc8d3eed7fef4ce795

Valentin, can you check this issue when sending an email from the webmail Earthlink after your patches in bug 1275746.
Blocks: 1275746
Status: UNCONFIRMED → NEW
status-firefox49: --- → unaffected
status-firefox50: --- → affected
status-firefox51: --- → affected
status-firefox52: --- → affected
tracking-firefox50: --- → ?
tracking-firefox51: --- → ?
tracking-firefox52: --- → ?
Component: Untriaged → Networking
Ever confirmed: true
Flags: needinfo?(kjb1611mikeiv) → needinfo?(valentin.gosu)
Keywords: regression
Product: Firefox → Core
(Assignee)

Comment 5

11 months ago
Those patches ensure that constructed URLs have a hostname.
It is possible that we are not more strict than other browsers, or that the web page is creating different URLs based on the user agent. So far I wasn't able to find what the offending URL is. I'll keep looking.
(Assignee)

Comment 6

11 months ago
It seems that the message comes from our own form validation logic, not the web app:
http://searchfox.org/mozilla-central/source/dom/locales/en-US/chrome/dom/dom.properties#35

I've looked around the code for the web mail client, and found this:

<input id="linkInputURL" value="http://" size="50" type="url">

The problem is, that http:// is not a valid URL, and will fail the form validation.
Flags: needinfo?(valentin.gosu)

Updated

11 months ago
Keywords: site-compat
(In reply to Valentin Gosu [:valentin] from comment #6)
> It seems that the message comes from our own form validation logic, not the
> web app:
> http://searchfox.org/mozilla-central/source/dom/locales/en-US/chrome/dom/dom.
> properties#35
> 
> I've looked around the code for the web mail client, and found this:
> 
> <input id="linkInputURL" value="http://" size="50" type="url">
> 
> The problem is, that http:// is not a valid URL, and will fail the form
> validation.

Do other browser get a different url or the same and they are just less strict? In later case we should revert our change?
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 8

11 months ago
For some reason other browsers don't seem to validate that form input. I changed its value to &^%$ and it still worked in chrome.
I don't know if that's because it's hidden, or for some other reason.
Flags: needinfo?(valentin.gosu)

Comment 9

11 months ago
Tracked for 50 since it seems to be a new regression.
tracking-firefox50: ? → +
(Assignee)

Comment 10

11 months ago
(In reply to Valentin Gosu [:valentin] from comment #8)
> For some reason other browsers don't seem to validate that form input. I
> changed its value to &^%$ and it still worked in chrome.
> I don't know if that's because it's hidden, or for some other reason.

NI :baku who seems to have some contributions to HTMLFormElement.

I tested with Chrome, and it seems it also fails to submit a form that contains
<input type="url" value="http://">
I don't know the reason Chrome doesn't validate the URL input in the web client, but it might have something to do with its visibility. Is there anything in the spec that would suggest that? Or is it a fluke that all other browsers don't validate that input?
Flags: needinfo?(amarchesini)

Comment 11

10 months ago
Track 51+ as regression.
tracking-firefox51: ? → +

Updated

10 months ago
Assignee: nobody → valentin.gosu
Whiteboard: [necko-active]
(Assignee)

Comment 12

10 months ago
Any opinions on comment 10?
Flags: needinfo?(bugs)

Comment 13

10 months ago
Valentin, do you have some minimal testcases for this?
Flags: needinfo?(bugs) → needinfo?(valentin.gosu)

Comment 14

10 months ago
Created attachment 8802242 [details]
test

When using FF or Chrome I get similar 'Please enter a URL' message when clicking reportValidity

Comment 15

10 months ago
I mean when value is http://

Comment 16

10 months ago
Created attachment 8802246 [details]
v2, with display: none

This shows the message isn't shown in Chrome with display: none;

Comment 17

10 months ago
Same with visibility: hidden

Comment 18

10 months ago
Note, the spec lets UAs to do basically whatever.
(Assignee)

Comment 19

10 months ago
Thanks for the investigating, Olli. Is there anything we can we do here?
Do we want to change our behaviour, or should we just push the website operator to fix their code?
Flags: needinfo?(valentin.gosu)
Flags: needinfo?(amarchesini)

Comment 20

10 months ago
I think we should fix our behavior.
The relevant code is around http://searchfox.org/mozilla-central/rev/d96317a351af8aa78ab9847e7feed964bbaac7d7/browser/modules/FormSubmitObserver.jsm#98
So somewhere there one should check at least if display is "none" or visibility "hidden".

But it isn't clear to me whether we should backout the regressing patch from FF50. 
That is of course the default action we should take, but is modifying FormSubmitObserver safe enough this late in beta?
Gijs might have an opinion.
Flags: needinfo?(gijskruitbosch+bugs)

Comment 21

10 months ago
Gijs pointed out that it isn't still clear why the page breaks even if the popup is shown.

Comment 22

10 months ago
Based on blink source code, they seem to use focusability as a check whether to show the popup.
https://chromium.googlesource.com/chromium/src/+blame/master/third_party/WebKit/Source/core/html/HTMLFormElement.cpp#240

Comment 23

10 months ago
(In reply to Olli Pettay [:smaug] from comment #21)
> Gijs pointed out that it isn't still clear why the page breaks even if the
> popup is shown.

Whether or not the popup is shown doesn't seem to affect here.
If I just modify FormSubmitObserver.jsm to return early, the issue still there.
(this should prevent also trying to focus some element)
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 24

10 months ago
I'm looking into backing out the patches. The backouts apply rather well, but I do have some odd test failures that don't happen locally:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c4b4915e73a&selectedJob=29405828
This makes me worry about potential regressions.
I'm not sure the situation with the Presentation API. S-C?
Flags: needinfo?(schien)
Tracking 52+, especially if we are considering backing out patches.
tracking-firefox52: ? → +
I spoke with S-C via email and he said "I think the modification is not related to the intermittent failure of presentation api tests. Feel free to try land it on inbound."

Valentin, do you have other concerns about potential regressions?
Flags: needinfo?(schien) → needinfo?(valentin.gosu)

Comment 28

10 months ago
I noted on IRC to Olli at the time that the "problem" seems to be that we validate when JS calls form.submit() and Chrome does not. No idea what the spec says on that matter, but could we align on that? AIUI changing the URL parsing behaviour will affect many other things, whereas the current behaviour (with the patch in) as far as validating URIs is concerned does match Chrome's - it's just that we use the result differently.

Comment 29

10 months ago
(In reply to :Gijs Kruitbosch (out til 26th, no reviews) from comment #28)
> I noted on IRC to Olli at the time that the "problem" seems to be that we
> validate when JS calls form.submit() and Chrome does not. No idea what the
> spec says on that matter, but could we align on that? AIUI changing the URL
> parsing behaviour will affect many other things, whereas the current
> behaviour (with the patch in) as far as validating URIs is concerned does
> match Chrome's - it's just that we use the result differently.

Specifically, see this testcase: http://jsbin.com/kicokihaju/1/edit?html,output

which submits in Chrome but doesn't in Firefox.
(Assignee)

Comment 30

10 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3edb7d78863
(Assignee)

Comment 31

10 months ago
(In reply to Andrew Overholt [:overholt] from comment #27)
> I spoke with S-C via email and he said "I think the modification is not
> related to the intermittent failure of presentation api tests. Feel free to
> try land it on inbound."

Actually, I just tracked down the issue, and it's an issue with the tests.
in test_presentation_dc_sender.html::testConstructRequestError() we do request = new PresentationRequest("\\\\\\"); which we expect to throw.
With the backout it doesn't throw as the URI resolves to resolves to http:///

> Valentin, do you have other concerns about potential regressions?

I am not too concerned about regressions, we had this URL parsing behaviour for a very long time.
But I would like to land it again as soon as possible, once we fix the form validation bug.
I'll land on inbound as soon as the try run finishes.
(Assignee)

Comment 32

10 months ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe38624aa14f6f1769f5d1e049e97ee89e119e34
Bug 1305204 - (Part 1) Backout bug 1275746 a=backout

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba408780dbcd994e10168394ede15795e8e5587b
Bug 1305204 - (Part 2) Backout bug 1275746 a=backout

https://hg.mozilla.org/integration/mozilla-inbound/rev/503de2e3054d20d8cc5825379c8d770c96968b7e
Bug 1305204 - (Part 3) Backout bug 1275746 a=backout

https://hg.mozilla.org/integration/mozilla-inbound/rev/a89de39a358784f56b19dfb3bfd32727ab102d42
Bug 1305204 - (Part 4) Comment out test with pseudo-bad URL r=me

Comment 33

10 months ago
backoutbugherder
https://hg.mozilla.org/mozilla-central/rev/fe38624aa14f
https://hg.mozilla.org/mozilla-central/rev/ba408780dbcd
https://hg.mozilla.org/mozilla-central/rev/503de2e3054d
https://hg.mozilla.org/mozilla-central/rev/a89de39a3587
Status: NEW → RESOLVED
Last Resolved: 10 months ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
(In reply to Valentin Gosu [:valentin] from comment #31)
> (In reply to Andrew Overholt [:overholt] from comment #27)
> > I spoke with S-C via email and he said "I think the modification is not
> > related to the intermittent failure of presentation api tests. Feel free to
> > try land it on inbound."
> 
> Actually, I just tracked down the issue, and it's an issue with the tests.
> in test_presentation_dc_sender.html::testConstructRequestError() we do
> request = new PresentationRequest("\\\\\\"); which we expect to throw.
> With the backout it doesn't throw as the URI resolves to resolves to http:///
> 
> > Valentin, do you have other concerns about potential regressions?
> 
> I am not too concerned about regressions, we had this URL parsing behaviour
> for a very long time.
> But I would like to land it again as soon as possible, once we fix the form
> validation bug.
> I'll land on inbound as soon as the try run finishes.

Thanks @valentin for figuring out the relationship. There is a similar test case in web platform test, I can remove the test step from mochitest to avoid duplication.
(Reporter)

Comment 35

10 months ago
Now that this issue is marked fixed as of version 52, is it safe for me to remove the email address? Or at least change the password to something more secure and it can then be given if needed for any future tests upon request? Or will it still be currently needed for an amount of time to come?

Thanks, Mike

Comment 36

10 months ago
Hi Mike,
could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(kjb1611mikeiv)

Comment 37

10 months ago
(In reply to :Gijs Kruitbosch (out til 26th, no reviews) from comment #28)
> I noted on IRC to Olli at the time that the "problem" seems to be that we
> validate when JS calls form.submit() and Chrome does not.

Oh, you changed the testcase now (since I couldn't reproduce the difference before, as I said on IRC). JS calls to form.submit() should and does not trigger validation in either browser().
It is that apparently if submit button tries to submit first in onclick, we re-trigger submit during default handling in such way that validation is done.

Changing this in beta is way too risky.
Once we've verified, we'll need to uplift the backouts to beta (I confirmed with smaug that's what he meant in comment 37).
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Comment 42

10 months ago
(In reply to Gerry Chang [:gchang] from comment #36)
> Hi Mike,
> could you please verify this issue is fixed as expected on a latest Nightly
> build? Thanks!

Also, although I don't completely understand what it's used for, I think the webmail code at earthlink should also be changed. 'http://' is not really a valid URL, and even though it led to finding this web-compatibility issue, it still feels quite hacky to me.
Flags: needinfo?(valentin.gosu)
(Assignee)

Comment 43

10 months ago
Comment on attachment 8804060 [details]
Bug 1305204 - (Part 3) Backout bug 1275746 a=backout

Approval Request Comment
[Feature/regressing bug #]: bug 1275746 - it actually exposed a preexisting bug.
[User impact if declined]: Triggers form validation bug on webmail.earthlink.net
[Describe test coverage new/current, TreeHerder]: Reverts to previous behaviour. Unit tests have been updated.
[Risks and why]: Low risk. It reverts to previous behaviour.
[String/UUID change made/needed]: None.

Note: landed patches can be uplifted to aurora. Reviewboard patches have been rebased for beta.
Attachment #8804060 - Flags: approval-mozilla-beta?
Attachment #8804060 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 44

10 months ago
(In reply to Gerry Chang [:gchang] from comment #36)
> Hi Mike,
> could you please verify this issue is fixed as expected on a latest Nightly
> build? Thanks!

Test verified on nightly 52.0a1 (2016-10-24) (64-bit)
The test e-mail was sent successfully and the behaviour of the original bug report does not happen.
Flags: needinfo?(kjb1611mikeiv)
makes sense to uplift this backout to 50 to me.

Comment 46

10 months ago
(In reply to Olli Pettay [:smaug] (reviewing overload, queue closed for a day or two) from comment #37)
> (In reply to :Gijs Kruitbosch (out til 26th, no reviews) from comment #28)
> > I noted on IRC to Olli at the time that the "problem" seems to be that we
> > validate when JS calls form.submit() and Chrome does not.
> 
> Oh, you changed the testcase now (since I couldn't reproduce the difference
> before, as I said on IRC). JS calls to form.submit() should and does not
> trigger validation in either browser().

Yes, sorry. When I looked back at IRC logs I found your message that said Firefox and Chrome behaved identically - I must have missed this at the time. I poked at it more and found the difference in the new testcase. Apologies, I should have clarified this in my comment.

> It is that apparently if submit button tries to submit first in onclick, we
> re-trigger submit during default handling in such way that validation is
> done.
> 
> Changing this in beta is way too risky.

Makes sense.
(Reporter)

Comment 47

10 months ago
(In reply to Valentin Gosu [:valentin] from comment #42)
> Also, although I don't completely understand what it's used for, I think the
> webmail code at earthlink should also be changed. 'http://' is not really a
> valid URL, and even though it led to finding this web-compatibility issue,
> it still feels quite hacky to me.

I sent a "Customer Support" e-mail to them about this citing this bugzilla progress page. I do not know for sure it will get to the right place or people needed to make any changes or choices about this. I am not sure if there is a process whereby Mozilla can make official contact with Earthlink to make contact with such a person and see this change made on that end. If there is, please have someone make such a contact. Especially since Firefox may not be the only browser to ever see this issue and their support staff will not know how to fix this or give a proper suggestion on how to avoid it.

Updated

10 months ago
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified

Comment 48

10 months ago
Comment on attachment 8804060 [details]
Bug 1305204 - (Part 3) Backout bug 1275746 a=backout

Patch backout was verified on Nightly and this seems like a new regression in 50, Aurora51+, Beta50+
Attachment #8804060 - Flags: approval-mozilla-beta?
Attachment #8804060 - Flags: approval-mozilla-beta+
Attachment #8804060 - Flags: approval-mozilla-aurora?
Attachment #8804060 - Flags: approval-mozilla-aurora+

Comment 49

10 months ago
uplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/0ab4ccecf1f8
status-firefox51: affected → fixed

Comment 50

10 months ago
uplift
https://hg.mozilla.org/releases/mozilla-beta/rev/1fb1c935f887
status-firefox50: affected → fixed
(Reporter)

Comment 51

10 months ago
Changing e-mail password now but not deleting account. If access is needed for testing in the future, contact me and I will change the password and let you know what it is again. - Mike
(Assignee)

Updated

9 months ago
Blocks: 1319078

Updated

2 months ago
See Also: → bug 1373327
You need to log in before you can comment on or make changes to this bug.