The default bug view has changed. See this FAQ.

[splitwindow] edit commands not available to extensions window (cut, copy, paste, select all, arrow keys navigation)

RESOLVED FIXED in mozilla1.8beta5

Status

()

Core
DOM
--
major
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: François Gagné, Assigned: bz)

Tracking

({fixed1.8, regression})

Trunk
mozilla1.8beta5
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.8b5 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050928 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050928 Firefox/1.6a1

The preference window of Adblock don't have working edit and navigation commands

Reproducible: Always

Steps to Reproduce:
1. Install Adblock
2. Do a Ctrl+Shift+P to get the preferences

Actual Results:  
Unable to Paste, navigate with the arrow keys.


If you change to another application and go back to Firefox the Paste and
navigation with arrow keys works.
(Reporter)

Updated

12 years ago
Blocks: 296639
Keywords: regression
Summary: [splitwindow] edit commands not available to extensions window (cut, copy, paste, select all, arrow keys navigation) → [splitwindow] edit commands not available to extensions window (cut, copy, paste, select all, arrow keys navigation)

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 1

12 years ago
I can confirm this on the latest branch nightly.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b5) Gecko/20050929 Firefox/1.4
(Reporter)

Comment 2

12 years ago
Additionnal detail, I'm talking about Adblock Plus:
http://www.extensionsmirror.nl/index.php?showtopic=774
I posted that already in Bug 220900 comment 134:
https://bugzilla.mozilla.org/show_bug.cgi?id=220900#c134
The adblock code does:

  function adblockSettings() {
      var settingsHandle = 
        window.open("chrome://adblock/content/settings.xul", 
                    "adblockPreferences",
                    "chrome,resizable,centerscreen,close=no");
      settingsHandle.focus();
  }

I bet that focus() call is the problem.  Can someone who can reproduce this bug
try removing that (the file is adblock.js, in the adblock.jar JAR) and seeing
whether that helps?

If that helps, I suspect this is not a splitwindow issue.  For that matter, is
there any evidence that this is caused by splitwindow?
If this is a regression, when did it regress?
(In reply to comment #5)
> If this is a regression, when did it regress?

1.8b4_2005073013 - 1.8b4_2005073111 (splitwindow indeed).
OK... So can you try removing that focus() call and seeing what happens?
(Reporter)

Comment 8

12 years ago
definitively a regression of splitwindow because it followed (to my memory) the
timeline of Comment #54 of bug #305032, also comment #1 and comment #2 in bug
#307355.
(In reply to comment #7)
> OK... So can you try removing that focus() call and seeing what happens?
Yess! It helps.
What if you replace "settingsHandle.focus();" with "var foo =
settingsHandle.focus"?  Is the bug still around?

That is, is the problem getting the "focus" property, or calling the "focus"
method?  My bets are on the former...
So for what it's worth, I do see the lookup of the focus property causing a
problem here.  So while the patch in bug 305032 helps for the case when
arguments attachment confusing things, it seems that we still have a problem. 
Aaron, Brian, what window, if any, _should_ have focus when this is all done? 
It looks like we're focusing the toplevel XUL window here, which seems
reasonable all things considered...
Flags: blocking1.8b5?
(Reporter)

Comment 12

12 years ago
>What if you replace "settingsHandle.focus();" with "var foo =
settingsHandle.focus"?  Is the bug still around?

No it works fine.

Comment 13

12 years ago
(In reply to comment #11)
> So for what it's worth, I do see the lookup of the focus property causing a
> problem here.  So while the patch in bug 305032 helps for the case when
> arguments attachment confusing things, it seems that we still have a problem. 
> Aaron, Brian, what window, if any, _should_ have focus when this is all done? 
> It looks like we're focusing the toplevel XUL window here, which seems
> reasonable all things considered...

It's a mistake to focus the top level XUL window itself. You want to focus the
first focusable control within it, which happens automatically at least for a
<dialog>.

I suppose we should fix it so that if you do focus a XUL window it makes sure
that one of its child controls is focused, and if not, focuses one.
> It's a mistake to focus the top level XUL window itself.

On our part, or adblock's?

> I suppose we should fix it so that if you do focus a XUL window it makes sure
> that one of its child controls is focused, and if not, focuses one.

Adblock is calling focus() before the XUL has loaded, so that's not an option in
this case....

Comment 15

12 years ago
not a blocker. let's look at the trunk for doing anything better here.
Flags: blocking1.8b5? → blocking1.8b5-
Created attachment 197904 [details] [diff] [review]
Could this help?

Blake, can you reproduce this bug?  If so, does this patch help?
Attachment #197904 - Flags: review?(mrbkap)
Comment on attachment 197904 [details] [diff] [review]
Could this help?

This seems to work for me (I think I can reproduce and this patch seemed to fix
it). I'll review it either tonight or tomorrow.
Comment on attachment 197904 [details] [diff] [review]
Could this help?

r=mrbkap
Attachment #197904 - Flags: review?(mrbkap) → review+
Attachment #197904 - Flags: superreview?(jst)
Comment on attachment 197904 [details] [diff] [review]
Could this help?

sr=jst
Attachment #197904 - Flags: superreview?(jst) → superreview+
Comment on attachment 197904 [details] [diff] [review]
Could this help?

Requesting 1.8b5 approval.  This changes the behavior of window.focus() only
for toplevel chrome windows that have not yet loaded their XUL.  It makes the
behavior match what we had before splitwindow.	I think this should be pretty
safe, but I wouldn't bet my life on it... ;)

If this gets approved, could someone please check it in?  I won't be able to
until late tomorrow afternoon.
Attachment #197904 - Flags: approval1.8b5?

Comment 21

12 years ago
Comment on attachment 197904 [details] [diff] [review]
Could this help?

blake, can you land this for BZ tonight?
Attachment #197904 - Flags: approval1.8b5? → approval1.8b5+
Fix checked in. MOZILLA_1_8_BRANCH and trunk.
Assignee: general → bzbarsky
Keywords: fixed1.8
Target Milestone: --- → mozilla1.8beta5

Updated

12 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.