33021 - nsXULWindow::GetContentShellById() fails to return nsIWebShell pointer

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P3)

Description

[Becki Sanford]

From Bugzilla Helper:
User-Agent: Mozilla/4.61 [en] (X11; U; Linux 2.2.12-20smp i686)
BuildID:    Milestone M14

nsXULWindow::GetContentShellById() fails to return nsIWebShell pointer when
element exists in XUL

Reproducible: Always
Steps to Reproduce:
1. Define the following element:
<html:iframe id="foo" type="content">
2. nsCOMPtr<nsIWebShell> content;
   rv = wsw->GetContentShellById("foo",getter_AddRefs(content));


Actual Results:  variable content is null

Expected Results:  expect variable content to reference <html:iframe>

I noticed that my nsWebShellWindow::GetContentShellById()
call no longer works in M14.  Tracking it down, it
appears nsHTMLFrameInnerFrame::CreateWebShell() in
nsFrameFrame.cpp saves the "type" attribute instead of
the "id" attribute when calling ContentShellAdded (a new
parameter added in M14).  That member function saves
the data to the "id" member of nsContentShellInfo object
(nsXULWindow.h).  When nsXULWindow::GetContentShellById()
is invoked, it compares aID with the saved "type".  This
appears to be a mismatch.

I temporarily changed the parameter I'm sending to
nsWebShellWindow::GetContentShellById() to use the
corresponding "type" attribute instead and the code
works fine now.

Upon further inspection, I see the code would work
if the "type" and "id" attributes of the <browser>
or <html:iframe> both start with the same text (as
in the case with navigator.xul).  However,
CreateWebShell() would have to be modified to save
off just the first 7 characters (it's already doing
this to check the type).  The function currently
saves the ENTIRE string...

Comment 1

[Gervase Markham [:gerv]]

[e-mail] - could you also have another look at this one? I have 
changed your Bugzilla permissions so, in future, bugs you submit get Confirmed 
immediately, and therefore should get more attention sooner. My apologies that 
these were left for so long.

Gerv

Comment 2

[Asa Dotzler [:asa]]

setting as New

Comment 3

[Becki Sanford]

Sorry I've been out of touch and unable to test the bug I reported.  An
untimely appendectomy took me out of commision for a month.  I'm
back to my PC so I'll check my bug against M16 and report back to you
as soon as I can.

--- becs

Comment 4

[Becki Sanford]

I just finished looking at nsHTMLFrameInnerFrame::CreateDocShell() in
layout/html/document/src/nsFrameFrame.cpp for M16.  The call to
ContentShellAdded() has not changed since M15 so the problem still exists.  The
3rd parameter to nsXULWindow::ContentShellAdded() should be the element's ID and
not its type (which the variable "value" represents in CreateDocShell).  The
call should be changed to something like the following:

  nsIAtom* idAtom = NS_NewAtom("id");
  nsAutoString idValue;
  if (NS_SUCCEEDED(parentContent->GetAttribute(kNameSpaceID_None, 
    idAtom, idValue))) {
    parentTreeOwner->ContentShellAdded(docShellAsItem,
      value.EqualsIgnoreCase("content-primary") ? PR_TRUE : PR_FALSE,
      idValue.GetUnicode());
  }

Comment 5

[Dawn Endico]

Reassigning all travis bugs to Valeski for triage. Don thought these were
all docshell related. Travis is no longer at netscape so these bugs are
unowned.

I'm reassigning 23 bugs to Valeski right now. To search for them,
search for "BUGSFORMERLYKNOWNASTRAVIS" in the description. The url below 
should work.

http://bugzilla.mozilla.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&email1=&emailtype1=substring&emailassigned_to1=1&email2=&emailtype2=substring&emailreporter2=1&bugidtype=include&bug_id=&changedin=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&short_desc=&short_desc_type=substring&long_desc=BUGSFORMERLYKNOWNASTRAVIS&long_desc_type=substring&bug_file_loc=&bug_file_loc_type=substring&status_whiteboard=&status_whiteboard_type=substring&keywords=&keywords_type=anywords&field0-0-0=noop&type0-0-0=noop&value0-0-0=&cmdtype=doit&newqueryname=&order=Reuse+same+sort+as+last+time

Comment 6

[leger]

Updating QA Contact

Comment 7

[Judson Valeski]

marking fixed. the webshell ID retreival mechanism has been reworked. this
method doesn't exist anymore.