63607 - input type=reset or submit require the optional value attribute

Status: VERIFIED FIXED

(Core Graveyard :: Embedding: APIs, defect, P2)

Description

[Tracy Walker [:tracy]]

seen on embed builds:

Windows comm. 2000-12-22 embed-win32.zip
Linux comm. 2000-12-22 embed-i686-pc-linux-gnu.tar.gz

steps to reproduce

attempts to open this page in an embed browser do nothing

the page should open with a menu screen to select choices for the test

Comment 1

[Conrad Carlen (not reading bugmail)]

Can you check that URL again? If that URL is right, what should happen is that
an Alert should come up saying "jrgm.mcom could not be found. Please check the
name and try again." That's what happens for me here with NS6. (I'm not there
but I'm behind the firewall). winEmbed doesn't implement nsIPrompt so you never
see the alert.

Comment 2

[Daniel (Leaf) Nunes]

reducing severity; this shouldn't hold the entire tree.

Comment 3

[Tracy Walker [:tracy]]

corrected URL....oh and this page first calls up a password dialogue.... to run 
this test without being logged in, goto 
http://jrgm.mcom.com/page-loader/loader.pl
both of these have worked fine for me with trunk builds.  But when I try it 
with the embed product it won't run.

Comment 4

[Tracy Walker [:tracy]]

with todays windows winEmbed & linux gtkEmbed builds I can get the page to load 
now but there isn't a submit querry button to run the test as seen with the 
commercial builds today.

Comment 5

[Adam Lock]

The gtkEmbed & winEmbed apps don't implement nsIPrompt in order to show the 
password prompt dialog so it's not surprising that this page is failing. The 
http channel is probably aborting the request when nsIPrompt cannot be found.

Is it a requirement that the embedding apps should implement this interface?

Comment 6

[Tracy Walker [:tracy]]

Adam, please read my comments from 12/26.  it is possible to access this test 
without logging in through the secure site.  now the problem is that the submit 
querry button that starts the test does not show in the embed product

Comment 7

[Adam Lock]

Okay, I understand now. Do you see it on both versions?

I am using the latest build and winEmbed shows the "Submit Query" button 
correctly.

Comment 8

[Adam Lock]

I see the problem now - it only occurs in the embedding build, not in the full 
build. Testing leads me to believe that this is another chrome packaging issue. 
It's possible to get the buttons to appear by building the modern skin & the 
global frontend but that pulls in a lot of other junk.

One thing noteworthy is that the HTML for the page in question is definitely 
"quirky" with form controls appearing in table cells, duff tags and other 
peculiarities. It could be that quirks mode isn't working correctly for 
embedding and fixing the page might be the quickest way to stop this bug being 
a blocker.

Comment 9

[Tracy Walker [:tracy]]

cc'ing John Morrison since the load test page is his.

Comment 10

[John Morrison]

Okay, I'm back from holidays ... 

> with form controls appearing in table cells

Um, which top 100 site does not place form controls inside table cells? :-]

> duff tags

Doh. Yeah, I was taking the quick and dirty route to disable some form 
controls by simply making them into "<xxxinput ...". But an invalid tag is 
simply ignored by the layout engine; as if it were not even in the page.


But at any rate, the real problem is triggered because I do not supply a 
'value' attribute for 'type="reset"' and 'type="submit"' input controls. 
However, that attribute is optional for those control types (in which case 
they just display the labels "Reset" and "Submit").

I've changed the page to supply a 'value' attribute for those controls, which 
works around this problem for now.

But there is still some sort of packaging issue that needs to be addressed.
I'll attach a simple test case.

Comment 11

[John Morrison]

Attachment: testcase; input type=reset and submit, with and without value attribute

Comment 12

[Adam Lock]

The "Submit Query" and "Reset" strings are localized and come from the 
HtmlForm.properties file in mozilla/layout/html/forms. It looks like the 
embedding version may not be seeing this for some reason even though it gets put 
into the dist directory.

Comment 13

[Adam Lock]

Further investigation.

HtmlForms.properties is copied correctly to the dist chrome directory, but 
layout thinks it is part of the "communicator" package and places it in 
$DIST/chrome/comm/content/communicator assuming it will get jarred up later when  
mozilla/xpfe/communicator gets built. In embedding the mozilla/xpfe/communicator 
module doesn't get built so there is no "communicator" package.

So at runtime when a form is loaded, layout tries to load the default strings 
for the Submit/Reset buttons with the chrome URL 
"chrome://communicator/locale/layout/HtmlForm.properties". Chrome can't resolve 
this onto a local file because "communicator" package doesn't exist. Chrome then 
tries the "wacky default" path which doesn't work either so the string bundle is 
not loaded. Therefore the HTML form buttons don't load their default strings.

I think the solution to this is that layout stuff should install into it's own 
package (e.g. "layout") just like necko stuff goes into the "necko" package. 
Layout shouldn't rely on the "communicator" package because that is for general 
stuff for the communicator suite of apps.

Comment 14

[Tracy Walker [:tracy]]

seen on windows and linux embed bits for 01/02/01

I am able to open the page loader page now and the submit and reset buttons are 
present.  starting the test with the submit button lands me on the dummy page 
(did I just say that??  hehe!!)  anyway, with windows, hitting reload, as the 
page suggests, does nothing and hitting reload on linix is not possible since 
there isn't a tool bar as of yet.

Comment 15

[John Morrison]

Yes, but it's a different bug than this one. I filed bug 63999 for the failure 
to continue loading pages. It conks out on JS 'document.cookie' -- maybe I can 
work around this, but I'd have to accept getting less information if I can't 
use cookies. 

Since 'document.cookie' needs to work for many other uses of the embed app
than just mine, I wanted to see whether the fix for that was scheduled to 
happen sooner rather than later.

Comment 16

[Paul Wyskoczka]

Updating QA Contact

Comment 17

[Tracy Walker [:tracy]]

with emebd builds for 2001-01-17 on windows and linux I am able to get the page 
to load and start the test.... how ever on windows the test stalls on the first 
page and linux stalls on a few pages into the test. I believe that behavior is 
bug 63999.

marking this bug fixed

Comment 18

[John Morrison]

Um, no. Check the test case -- the submit and reset form controls do not
appear unless you provide a "value" attribute. 

(I modified the original form to provide these attributes, so the buttons 
now show up in that example).

Comment 19

[Judson Valeski]

-> adam

Comment 20

[Judson Valeski]

for real this time. -> adam

Comment 21

[Tracy Walker [:tracy]]

this is working on both windows and linux embed builds

Comment 22

[Adam Lock]

The bug still exists so I'm reopening. The form HTML has been hardcoded with the 
"reset" & "submit" values so it just looks like its fixed. The bug still remains 
that the embedding nightlies don't pull in HtmlForm.properties because it is 
in the "communicator" chrome package and not the "global" one where it should 
be.

Comment 23

[Adam Lock]

It looks as though some significant changes should be done to the layout chrome 
to fix this satisfactorily.

1. mozilla/layout should gain a resources directory, just like
   mozilla/netwerk/resources
2. HtmlForm.properties and ImageDocument.properyies should move from
   layout/html/forms/src into mozilla/layout/resources/locale/en-US
3. Create a contents.rdf naming a new "layout" package and place in
   mozilla/layout/resources/locale/en-US
4. Create/modify makefiles & jar.mn to publish files to locale/en-US/layout
5. Replace all references to the old chrome URL with the new one, e.g.
   "chrome://layout/locale/HtmlForm.properties"
6. Update language packs (involves a lot of moving code around).
7. Update mozilla/embedding/config manifest to include HtmlForm.properties

Comment 24

[Adam Lock]

Marking 0.9.1.

Comment 25

[Michael Dunn]

Correction: Changing QA contact for the Embed API bugs to David Epstein.

Comment 26

[Adam Lock]

Patch for bug 90749 should fix this. I will close this bug off when the other
one is and raise a bug on the HTML form folks to move their strings out of
communicator and into global or a new package called layout.

Comment 27

[Adam Lock]

Fixed by bug 90749. Bug 111339 opened to cover reworking how and where layout
stores its chrome.

Comment 28

[David Epstein]

Tested on 11/28 MfcEmbed and testGtkEmbed builds. Reset & Submit works fine for
page load testing.