Closed Bug 409481 Opened 12 years ago Closed 12 years ago

XML Parsing error if you open a bookmark in the sidebar

Categories

(Firefox :: General, defect, major)

defect
Not set
major

Tracking

()

VERIFIED FIXED
Firefox 3 beta3

People

(Reporter: wgianopoulos, Assigned: rflint)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

The code checked in for bug 406330 causes an XML parsing error for bookmarks opened in the sidebar.

Full text of error is:

XML Parsing Error: undefined entity
Location: chrome://browser/content/web-panels.xul
Line Number 121, Column 7:

      <menuitem id="context-back"
------^

reverting the browser-context.inc portion of the patch avoids the issue.

I wonder what any of the context menu changes are for anyway, as this was purportedly a patch to correct rtl issues on the toolbars.
Flags: blocking-firefox3?
It would also seem we need a unit test for opening bookmarks in the sidebar.
(In reply to comment #0)
> I wonder what any of the context menu changes are for anyway, as this was
> purportedly a patch to correct rtl issues on the toolbars.
> 
OIC.  I didn't realize icons had been added to the context menu under gnomestripe.
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → rflint
Status: NEW → ASSIGNED
Attachment #294318 - Flags: review?(gavin.sharp)
Flags: in-testsuite?
Duplicate of this bug: 409554
this is in the litmus FFT: https://litmus.mozilla.org/show_test.cgi?id=4123
Flags: in-litmus+
Comment on attachment 294318 [details] [diff] [review]
Patch

>Index: test/browser_bug409481.js

>+var gWm = Cc['@mozilla.org/appshell/window-mediator;1'].
>+          getService(Ci.nsIWindowMediator);
>+
>+function test() {
>+  waitForExplicitFinish();
>+  var win = gWm.getMostRecentWindow("navigator:browser");

Why are you using the window mediator? This test code runs in the browser window's scope already, so you should be able to just use openWebPanel directly, no?

>+  var url = 'data:text/html,<div%20id="test_bug409481">Content!</div>';
>+  win.openWebPanel("Sidebar Test", url);
>+
>+  setTimeout(runTest, 250);

A load listener would be more reliable, I think, but perhaps that won't be an issue in practice.

>+function runTest() {
>+  var win = gWm.getMostRecentWindow("navigator:browser");
>+
>+  var sidebar = win.document.getElementById("sidebar");

Similarly, should be no need for win here.
Attachment #294318 - Flags: review?(gavin.sharp) → review+
Attachment #294318 - Flags: approval1.9?
Comment on attachment 294318 [details] [diff] [review]
Patch

a=mconnor on behalf of drivers
Attachment #294318 - Flags: approval1.9? → approval1.9+
Duplicate of this bug: 409653
mozilla/browser/base/content/test/browser_bug409481.js 	1.1
mozilla/browser/base/content/test/Makefile.in 	1.6
mozilla/browser/base/content/web-panels.xul 	1.23

(In reply to comment #6)
> Why are you using the window mediator? 
Force of habit :)

> A load listener would be more reliable, I think, but perhaps that won't be an
> issue in practice.
Did that and ran into a situation similar to bug 310955 - so I've changed the test to use a little bit of both.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Can you attach an updated patch?
Attached patch As checked inSplinter Review
Attachment #294318 - Attachment is obsolete: true
Flags: blocking-firefox3? → blocking-firefox3+
Verified fix on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3) Gecko/2008020418 Firefox/3.0b3.  
Status: RESOLVED → VERIFIED
No longer blocks: 406330
Depends on: 421333
You need to log in before you can comment on or make changes to this bug.