WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
92001
[EFL][WK2] Update uri when the active URI is changed while loading.
https://bugs.webkit.org/show_bug.cgi?id=92001
Summary
[EFL][WK2] Update uri when the active URI is changed while loading.
Ryuan Choi
Reported
2012-07-23 07:47:12 PDT
For now, ewk_view_uri_get just return the last uri of setter. Because uri can be changed while loading, the uri of ewk_view should be updated.
Attachments
Patch
(8.45 KB, patch)
2012-07-23 08:13 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(9.58 KB, patch)
2012-07-23 15:48 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(9.77 KB, patch)
2012-07-24 00:58 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(9.97 KB, patch)
2012-07-24 01:32 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Ryuan Choi
Comment 1
2012-07-23 08:13:09 PDT
Created
attachment 153800
[details]
Patch
Chris Dumez
Comment 2
2012-07-23 08:34:38 PDT
Comment on
attachment 153800
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=153800&action=review
> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:548 > + if (priv->uri && activeURI == priv->uri)
This check becomes useless if we use the output of eina_stringshare_replace. See below.
> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:551 > + eina_stringshare_replace(&priv->uri, activeURI.data());
eina_stringshare_replace returns a boolean. You can do: if (!eina_stringshare_replace(&priv->uri, activeURI.data())) return;
> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.h:27 > +#define TEST_PAGE_URI(html) ("file://"TEST_RESOURCES_DIR"/"html)
How about using a static method in EWK2UnitTestBase instead? Also, I think the name should be something like "pathForResource(const char* resource);". The Resource may not be an HTML page, it may be an icon, a pdf file, ...
Ryuan Choi
Comment 3
2012-07-23 15:48:14 PDT
Created
attachment 153884
[details]
Patch
Ryuan Choi
Comment 4
2012-07-23 15:53:40 PDT
(In reply to
comment #2
)
> (From update of
attachment 153800
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=153800&action=review
> > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:548 > > + if (priv->uri && activeURI == priv->uri) > > This check becomes useless if we use the output of eina_stringshare_replace. See below. > > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:551 > > + eina_stringshare_replace(&priv->uri, activeURI.data()); > > eina_stringshare_replace returns a boolean. You can do: > if (!eina_stringshare_replace(&priv->uri, activeURI.data())) > return; >
I fixed.
> > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestBase.h:27 > > +#define TEST_PAGE_URI(html) ("file://"TEST_RESOURCES_DIR"/"html) > > How about using a static method in EWK2UnitTestBase instead? Also, I think the name should be something like "pathForResource(const char* resource);". The Resource may not be an HTML page, it may be an icon, a pdf file, ...
I agree that pathForResource is better name. And I used macro to avoid string manipulation and free, but it's not important. I added pathForResource like you mentioned. thank you for your comment.
Chris Dumez
Comment 5
2012-07-23 23:13:18 PDT
Comment on
attachment 153884
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=153884&action=review
> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:548 > + if (eina_stringshare_replace(&priv->uri, activeURI.data()))
Eh... Isn't it the opposite? :) I mean: if (!eina_stringshare_replace()) return;
> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestEnvironment.cpp:42 > +Eina_Stringshare* EWK2UnitTestEnvironment::pathForResource(const char* resource)
I would return a CString to avoid handling memory manually. CString is always commonly used in EFL tests.
Ryuan Choi
Comment 6
2012-07-24 00:58:44 PDT
Created
attachment 153981
[details]
Patch
Ryuan Choi
Comment 7
2012-07-24 00:59:57 PDT
(In reply to
comment #5
)
> (From update of
attachment 153884
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=153884&action=review
> > > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:548 > > + if (eina_stringshare_replace(&priv->uri, activeURI.data())) > > Eh... Isn't it the opposite? :) > > I mean: if (!eina_stringshare_replace()) return; >
Ah.. sorry, my mistake. I fixed. Thank you.
> > Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestEnvironment.cpp:42 > > +Eina_Stringshare* EWK2UnitTestEnvironment::pathForResource(const char* resource) > > I would return a CString to avoid handling memory manually. CString is always commonly used in EFL tests.
Done.
Chris Dumez
Comment 8
2012-07-24 01:22:08 PDT
Comment on
attachment 153981
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=153981&action=review
> Source/WebKit2/UIProcess/API/efl/tests/UnitTestUtils/EWK2UnitTestEnvironment.cpp:46 > + return makeString("file://"TEST_RESOURCES_DIR"/", resource).utf8();
If you prepend "file://" then the method name is wrong. This returns a URL, not a path.
Ryuan Choi
Comment 9
2012-07-24 01:32:10 PDT
Created
attachment 153986
[details]
Patch
Chris Dumez
Comment 10
2012-07-24 01:37:00 PDT
Comment on
attachment 153986
[details]
Patch LGTM.
Hajime Morrita
Comment 11
2012-07-25 00:53:03 PDT
Comment on
attachment 153986
[details]
Patch rs=me.
WebKit Review Bot
Comment 12
2012-07-25 02:04:17 PDT
Comment on
attachment 153986
[details]
Patch Clearing flags on attachment: 153986 Committed
r123591
: <
http://trac.webkit.org/changeset/123591
>
WebKit Review Bot
Comment 13
2012-07-25 02:04:23 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug