538999 - Formfill does not zoom out on load of dynamically loaded page content

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aakash Desai [:aakashd]]

Build Id:

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2pre) Gecko/20100111 Firefox/3.6pre Fennec/1.1a1pre

and

Mozilla/5.0 (X11; U; Linux armv6l; Nokia N8xx; en-US; rv:1.9.2) Gecko/20100111 Firefox/3.6pre Fennec/1.1a1pre

Steps to Reproduce:
1. Go to www.aircanada.com
2. Click on "Canada - English"
3. Click on "Flight Status"
4. Tap on the flight number field next to "AC" and the date field.
5. Type in "757" as a search term.
6. Click on "Search"

Actual Results:
The page loads the content, but the formfill bar is still visible and the page is zoomed in.

Expected Results: 
The page should show the entire page or wherever the content has been updated.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

What is changing here is the content of an iframe in the webpage.

I'm starting to thing of correcting this one and bug 538430 in the same row by adding a custom web progress listener to the ownerDocument of the current target element.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #1)
> What is changing here is the content of an iframe in the webpage.
> 
> I'm starting to thing of correcting this one and bug 538430 in the same row by
> adding a custom web progress listener to the ownerDocument of the current
> target element.

Finally I have a plan relying on http://mxr.mozilla.org/mozilla-central/source/content/html/content/public/nsIFormSubmitObserver.idl for those.

I will post a working patch today or tomorrow.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

This patch correct this bug and bug bug 538430.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 428332 [details] [diff] [review]
Patch

>diff -r d61fdff66712 chrome/content/browser.js

>+const gFormSubmitObserver = {
>+  notify: function notify(aFormElement, aWindow, aActionURI, aCancelSubmit) {
>+    let doc = aWindow.content.top.document;
>+    let tab = Browser.getTabForDocument(doc);
>+    if (tab)
>+      tab.browser.hasChanged = true;
>+  },

Maybe we could just call FormHelper.close() here?

>-    if (location != this.browser.lastSpec) {
>+    if (location != this.browser.lastSpec || this.browser.hasChanged) {

Do we really want to fire all this code?

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

I guess we want to zoom out, but do we want to close notifications and scroll to top?

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #5)
> I guess we want to zoom out, but do we want to close notifications and scroll
> to top?

I'm not really sure for the notifications part, but this didn't remove the notification prompt for remembering password as example.

For the scroll to top part, I'd say yes, and as an example just tried to log in into twitter by using the form assistant(and futhermore, once we'll sync the browser and the content part we really want to)

So, i agree that tab.browser.hasChanged is not very elegant, but I've wanted to avoid duplicating code.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

Just a little changed to the patch to remove the hasChanged flag.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 442685 [details] [diff] [review]
Patch v0.2

>diff -r 64c363ad5a4d chrome/content/browser.js
> 
>+const gFormSubmitObserver = {

var FormSubmitObserver + {

We need to move away from Firefoxes old naming conventions and use the new ones.

Thanks for removing the flag!

r+ with nit fixed

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/e59e43d3d2d9