Patch to follow.
Created attachment 60884 [details] [PATCH] Proposed change.
Comment on attachment 60884 [details] [PATCH] Proposed change. WebCore/inspector/front-end/StoragePanel.js:52 + // TODO: uncomment when AppCache is implemented. Please put bug number after the TODO.
Comment on attachment 60884 [details] [PATCH] Proposed change. (In reply to comment #2) > (From update of attachment 60884 [details]) > WebCore/inspector/front-end/StoragePanel.js:52 > + // TODO: uncomment when AppCache is implemented. > Please put bug number after the TODO. There is no particular bug yet. But this is a live topic, so I expect this todo to vanish shortly.
Comment on attachment 60884 [details] [PATCH] Proposed change. Clearing flags on attachment: 60884 Committed r62946: <http://trac.webkit.org/changeset/62946>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/62946 might have broken SnowLeopard Intel Release (Tests)
FWIW, it is working enough on Safari/WebKit to having it enabled. Please change it so it stays enabled in WebKit, and Chrome disables it.
Checking a global variable or preference would work nicer. Something like in inspector.js: // TODO: eventually enable application cache WebInspector.applicationCacheEnabled = false; And then instead of commenting out the code, you just wrap it in a block around this variable. This means that when we want to enable the feature locally, adding new features and enhancements we only have to change this variable to true and revert it to false when making a patch. With this patch we have to uncomment a lot of code in different places (and comment it back when making patches). When finally enabling the feature we would just delete the variable and its checks (easily searchable).
(In reply to comment #7) > FWIW, it is working enough on Safari/WebKit to having it enabled. Please change it so it stays enabled in WebKit, and Chrome disables it. Timothy, I'd appreciate if we used the same process as while implementing Timeline. We make it work and then agree on enabling it. Michael was pointing out the problems that make the feature look incomplete. Like - start the app - go to Storage - click on appcache icon - click on localStore icon appcache page is not hidden. So the thing is really broken to be enabled.
(In reply to comment #9) > Like > - start the app > - go to Storage > - click on appcache icon > - click on localStore icon > > appcache page is not hidden. So the thing is really broken to be enabled. I have never encountered this problem. It works in Safari. If I saw this I would certainly have fixed it. Does an error get thrown in the inspector's inspector for Chrome? Maybe a check I missed if there was no data yet?
(In reply to comment #10) > (In reply to comment #9) > > Like > > - start the app > > - go to Storage > > - click on appcache icon > > - click on localStore icon > > > > appcache page is not hidden. So the thing is really broken to be enabled. > > I have never encountered this problem. It works in Safari. If I saw this I would > certainly have fixed it. Does an error get thrown in the inspector's inspector > for Chrome? Maybe a check I missed if there was no data yet? I tried it in Chromium, did not try in Safari. That is not the main point though.
(In reply to comment #9) > (In reply to comment #7) > > FWIW, it is working enough on Safari/WebKit to having it enabled. Please change it so it stays enabled in WebKit, and Chrome disables it. > > Timothy, I'd appreciate if we used the same process as while implementing Timeline. We make it work and then agree on enabling it. Michael was pointing out the problems that make the feature look incomplete. > > Like > - start the app > - go to Storage > - click on appcache icon > - click on localStore icon > > appcache page is not hidden. So the thing is really broken to be enabled. Was that in WebKit/Safari? I agree the feature should be hidden in Chrome is it isn't ready. Timeline panel was different, sicne it was a large new feature and could easily be hidden by removing the panel. This was multiple lines of code that got commented out and prevents easy developemt as Joe pointed out. I think this patch needs to be reverted or changed to check a global variable. And preferibly enabled in WebKit/Safari and disabled in Chrome until it is ready.
Also we don't normally land commented out code. We either remove it or hide it being a flag. So this patch should have not landed.
(In reply to comment #13) > Also we don't normally land commented out code. We either remove it or hide it being a flag. So this patch should have not landed. k, I am happy to hide it behind the preference flag. Will that be fine by you? I am actually a bit surprised seeing the reaction. My perception was that I am hiding it for you. As I understand this is change was copying Kavita's code, reworking it and landing it enabled without her permission / talking to her.
I guess the problem was on our end, we should have made sure it was disabled in Chrome when it got added to the UI. Similar to how V8 only features are done.
Yes a preference flag is fine, if it is true in TOT WebKit for Safari.
Created attachment 61807 [details] [PATCH] Take 2. Disabled in Chromium, enabled in WebKit.
Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/inspector/front-end/Settings.js M WebCore/inspector/front-end/StoragePanel.js M WebKit/chromium/ChangeLog M WebKit/chromium/src/js/DevTools.js Committed r63549