310241 - InvisionBoard AJAX/JS facility for quoting text no longer works

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[John Hanna]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050927 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050927 Firefox/1.4

A little bit of InvisionBoard AJAX/JS that used to work with 1.5 but now doesn't
work. It does still work with 1.07, Opera 8.5 and IE.
If you goto http://www.wow-factor.com/forums/index.php?showtopic=6197 scroll to
the bottom of the page you will see the Quick Reply box.
Highlight some text from a previous post and then hover your mouse back over the
Quick Reply box and you will get an option appearing to "Add Selected Text as Quote"

It doesn't add the text anymore.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050927 Firefox/1.4

Reproducible: Always

Steps to Reproduce:
1.goto http://www.wow-factor.com/forums/index.php?showtopic=6197
2.scroll to the bottom of the page you will see the Quick Reply box.
3.Highlight some text from a previous post
4.hover your mouse back over the Quick Reply box
5.you will get an option appearing to "Add Selected Text as Quote"
6.It doesn't add the text anymore.

Actual Results:  
Step 6. It doesn't add the text anymore.

Expected Results:  
Placed the highlighted text from the previous post into the textarea as a QUOTE

This worked in 1.5 Beta 1 but failed after ( what I believe ) the UPDATE to
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050927 Firefox/1.4

Note: It works fine in Firefox 1.07, Opera 8.5 and IE 6

Comment 1

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b5) Gecko/20050927
Firefox/1.4 ID:2005092714

Confirmed->NEW
?1.8b5

I'll check the regression date/time tomorrow.

Comment 2

[Peter van der Woude [:Peter6]]

*** Bug 310239 has been marked as a duplicate of this bug. ***

Comment 3

[Peter van der Woude [:Peter6]]

John, are you sure this ever worked ?
I tried builds back till before 1.5b1 (20050906 0739pdt) and none of them seem
to work.
Nothing in JSC / Errorconsole either.

Can you try some builds to figure it out ?

Comment 4

[John Hanna]

In reply to Peter Re builds
Peter this works as it should in FF 1.07 and I must apologise for not knowing
about earlier nightly builds as I just converted my 1.5 Beta 1 to nightly
updates a couple of days ago.
I am sure this function worked in 1.5 Beta 1

Cheers
John

Comment 5

[Michael K.]

Attachment: Simplified testcase

Comment 6

[John Hanna]

Just posted at Invisionboard and had this tested by users
They report that it doesn't work with 1.5 Beta 1 either.
John

Comment 7

[Keith]

I've tried the simplified testcase (Comment #5) back to 2005030107 (Trunk
nightly of March 1, 2005) with no success. Will try going back further later.

Comment 8

[Michael K.]

I believe I've found the cause for this. In 1.0.7, the text remains selected
when clicking on the button in the testcase. In the latest nightly builds the
text gets deselected (and thus window.getSelection() returns nothing), which I
believe is intended behaviour (although I can't find the bug number). Mark this
bug invalid?

Comment 9

[jonnunan]

There is a test case here that goes more in depth.
https://bugzilla.mozilla.org/show_bug.cgi?id=120776

INPUT type=button doesn't seem to work,
but <button> does.
focus also seems to be lost when clicking on text links, but not on images.

Comment 10

[PikeUK]

Is this bug Firefox specific or does it need to be moved somewhere more appropriate?

Comment 11

[Martijn Wargers (dead)]

This regressed between 2004-11-28 and 2004-12-02, possibly a change in behavior
from bug 269318?
For me, current behavior seems more logical to me, though.

Comment 12

[John Hanna]

(In reply to comment #11)
> This regressed between 2004-11-28 and 2004-12-02, possibly a change in behavior
> from bug 269318?
> For me, current behavior seems more logical to me, though.

(In reply to comment #10)
> Is this bug Firefox specific or does it need to be moved somewhere more
appropriate?

Well, it's not a bug in 1.07, Mozilla or any other modern browser, simply in the
version of 1.5 as detailed in the original post.

Comment 13

[Asa Dotzler [:asa]]

Aaron, can you look into this. Seems like a regression from your changes.

Comment 14

[Aaron Leventhal]

I don't see the "quick reply" box in the bottom of that page, so I am using the
"simplified testcase"

> it's not a bug in 1.07, Mozilla or any other modern browser, simply in the
> version of 1.5 as detailed in the original post.

What other browser does this test case work in? I can't get it to work in either
IE or Opera.

Comment 15

[Michael K.]

> What other browser does this test case work in? I can't get it to work in either
IE or Opera.

The testcase uses window.getSelection() only. This is a Mozilla only feature
(AFAIK). The actual InvisionBoard script checks whether window.getSelection()
exists, and if not uses either document.selection or document.getSelection().

Comment 16

[John Hanna]

(In reply to comment #15)
> > What other browser does this test case work in? I can't get it to work in either
> IE or Opera.
> 
> The testcase uses window.getSelection() only. This is a Mozilla only feature
> (AFAIK). The actual InvisionBoard script checks whether window.getSelection()
> exists, and if not uses either document.selection or document.getSelection().

Here is the IPB JS that covers this ( I think )
/*--------------------------------------------*/
// check selection
/*--------------------------------------------*/

function checkselection()
{
	var myselection = '';
	
	if ( window.getSelection )
	{
		myselection = window.getSelection();
	}
	else if ( document.selection )
	{
		myselection = document.selection.createRange().text;
	}
	else if ( document.getSelection )
	{
		myselection = document.getSelection();
	}
	
	if ( myselection != '' && myselection != null )
	{
		if ( myselection != mystored_selection )
		{
			document.getElementById('fastreply-pastesel').style.display = '';
			mystored_selection = myselection;
		}
	}
}

/*--------------------------------------------*/
// Paste selection
/*--------------------------------------------*/

function pasteselection()
{
	if ( mystored_selection != '' && mystored_selection != null )
	{
		document.getElementById('fastreplyarea').value +=
'[quote]'+mystored_selection+'[/quote]\n';
		//document.getElementById('fastreply-pastesel').style.display = 'none';
	}
}
/*============================*/

and the related  html...

<textarea onmouseover='checkselection()' id='fastreplyarea' cols="70" rows="8"
name="Post" class="textarea" tabindex="1"></textarea>
		<!-- HIDDEN PASTE SELECTION QUESTION --> 
		<div id='fastreply-pastesel' align='center' style='display:none;'>
		   <input type="button" name="pastesel" onclick="pasteselection();" value="Add
Selected Text As Quote" class="button" />
		</div>
		<!-- / HIDDEN PASTE SELECTION QUESTION -->

Comment 17

[Aaron Leventhal]

The changes that caused this are pretty baked in now, and I don't want to back
them out. It will cause other important fixes to regress. The way to fix this is
in nsEventStateManager, but I'm not sure a low risk fix can be made for Firefox 1.5.

As a workaround can you try grabbing selection in an onblur handler and storing
it for use in your script?

I realize a workaround is probably not what you want but please understand I'm
looking at this bug late in the cycle, and we've had a lot of problems getting
focus to the point where it is now.

Comment 18

[Aaron Leventhal]

Attachment: Use mLastFocusedWith as we used to, to determine whether or not to tab from focus or selection

Comment 19

[Asa Dotzler [:asa]]

bryner and johnny, this looks like something we want to fix. Can you help us
look into this? I'm scared of taking focus changes so late in the game but this
is a regression and may be widespread. 

Comment 20

[Mats Palmgren (inactive)]

Comment on attachment 198003 [details] [diff] [review]
Use mLastFocusedWith as we used to, to determine whether or not to tab from focus or selection

Looks good to me. With this patch we don't reset the selection
when right-clicking on a link, even when outside the selection -
personally I like that behaviour and the question of what to
show in the context menu is a separate issue (bug 135813).
r=mats

Comment 21

[Asa Dotzler [:asa]]

not a beta blocker. we'd consider a fully reviewed patch for approval if you all
can make the case that we're not taking on significant risk with this approach.

Comment 22

[╧ЫжыеТ]

(In reply to comment #14)
> I don't see the "quick reply" box in the bottom of that page, so I am using the
> "simplified testcase"

click fastREPLY button, then the box is show!

Comment 23

[Aaron Leventhal]

Fixed on trunk.

Comment 24

[Paulo Deleo]

Not fixed at Firefox 1.5 final.

Comment 25

[Adam Guthrie]

(In reply to comment #24)
> Not fixed at Firefox 1.5 final.

That's because the patch hasn't been checked into the MOZILLA_1_8_BRANCH.

Comment 26

[Aaron Leventhal]

(In reply to comment #25)
> (In reply to comment #24)
> > Not fixed at Firefox 1.5 final.
> 
> That's because the patch hasn't been checked into the MOZILLA_1_8_BRANCH.
> 

It was considered too risky.

Comment 27

[Daniel Veditz [:dveditz]]

denied for 1.8.0.2, the risk/benefit ratio hasn't changed

Comment 28

[Boris Zbarsky [:bzbarsky]]

*** Bug 317914 has been marked as a duplicate of this bug. ***

Comment 29

[Martijn Wargers (dead)]

*** Bug 332131 has been marked as a duplicate of this bug. ***

Comment 30

[Darin Fisher]

Comment on attachment 198003 [details] [diff] [review]
Use mLastFocusedWith as we used to, to determine whether or not to tab from focus or selection

please land this on the 1.8 branch and add the fixed1.8.1 keyword.

Comment 31

[Darin Fisher]

Please land this on the 1.8 branch ASAP so this does not hold up the FF2 beta1 release.  Thanks!

Comment 32

[Mike Schroepfer]

Moving to Beta2 and clearing approval flag.  We'll pick this up after beta1.  Aaron can you land once the tree re-opens.

Comment 33

[Aaron Leventhal]

Actually I would strongly prefer that Mats Palmgren land this since he owns focus code now, and was a reviewer of the patch. I'm not prepared to deal with regressions from this, or any other focus/keynav issues moving forward.

Comment 34

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 198003 [details] [diff] [review]
Use mLastFocusedWith as we used to, to determine whether or not to tab from focus or selection

a=drivers, mats, please land this on the branch

Comment 35

[Mats Palmgren (inactive)]

Checked in on MOZILLA_1_8_BRANCH at 2006-07-19 13:41 PDT:
content/events/src/nsEventStateManager.h     1.137.4.4
content/events/src/nsEventStateManager.cpp   1.595.2.22