414291 - Livejournal Insert/Edit URL button doesn't work. Pops up blank page.

Status: VERIFIED FIXED

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

Description

[Avuton Olrich]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9b3pre) Gecko/2008012008 Firefox/3.0b3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9b3pre) Gecko/2008012008 Firefox/3.0b3pre

Livejournal.com Insert/Edit button doesn't work, it pops up a blank page. My wife remembers specifically it working before, but obviously a change in the webpage or Firefox has changed. Have tested with the nightly from tonight for windows and linux (build identifier). Will attach picture illustrating the button that doesn't work in the URL above.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Avuton Olrich]

Attachment: This illustrates the button that after pressing doesn't work. Requires login to get there.

Comment 2

[Ria Klaassen (not reading all bugmail)]

Regression window is http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=2007-07-25+20%3A00&maxdate=2007-07-26+23%3A00
Caused by bug 280959?
I see the bug without login BTW.

Comment 3

[Ria Klaassen (not reading all bugmail)]

Wrong guess, it is bug 194404. Tested on Win XP and Vista.

Comment 4

[Martijn Wargers (dead)]

Attachment: testcase

I managed to minimize it to this.

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This is because they put "modal=yes" in the window.open feature string. IE ignores that whereas we open a modal dialog.

Comment 6

[Johnny Stenback (:jst)]

So the easiest way to fix this would be to parse out the modal=... argument out of the options string in nsGlobalWindow::OpenInternal(), unless aContentModal is true. Parsing stuff out is sort of lame and hairy, but I don't see an easier way here. bz, anything else come to mind here?

Comment 7

[Boris Zbarsky [:bzbarsky]]

As in, ignore modal="yes" like we used to for content dialogs, except those opened via showModalDialog()?  That sounds like the right approach to me...  It's not lame, and hopefully not even that hairy (since we used to do exactly that, except for all content cases), right?

Comment 8

[Johnny Stenback (:jst)]

bz, we used to ignore modal=yes in the window watcher code, but now we rely on that for communicating from the DOM code to the window watcher code that the window we're opening is a modal content window (i.e. showModalDialog()). To fix this bug w/o changing the showModalDialog()/window watcher interaction we'd need to parse the options argument in nsGlobalWindow::OpenInternal() and strip out modal=yes there. IOW one more options argument parser, that's the lame part.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Hmm.  That does seem lame, and more importantly fragile.

Could we stop piggybacking on the "modal" option, go back to ignoring it, and use a moz-modal-content-dialog option or some such to communicate with the windowwatcher?

Comment 10

[Johnny Stenback (:jst)]

Attachment: Fix.

This makes window.open() ignore modal=yes by reverting part of the changes we took when we added support for window.showModalDialog(), and makes the window.showModalDialog() code pass in a "-moz-internal-modal" option that the window watcher now has special code for.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 309305 [details] [diff] [review]
Fix.

Looks good.  Get a test in too?

Comment 12

[Johnny Stenback (:jst)]

Attachment: tests.

Comment 13

[Johnny Stenback (:jst)]

Fixed.

Comment 14

[Marcia Knous [:marcia]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b5pre) Gecko/2008031804 Minefield/3.0b5pre and the Win XP nightly. I used the Testcase in Comment 4 to verify.